Add source location URL to CI test failures.#311
Add source location URL to CI test failures.#311zainab-ali wants to merge 1 commit intotypelevel:mainfrom
Conversation
| } yield TestOutcome(name, end - start, res, logs) | ||
| } | ||
|
|
||
| def pure(name: String)(ex: () => Expectations): TestOutcome = { |
There was a problem hiding this comment.
We no longer call Test.pure in our own code, as we need an effect to get the source URL.
Perhaps we should deprecate it?
There was a problem hiding this comment.
I think it doesn't hurt to keep it either, I've no strong opinion.
|
|
||
| def formatted: Option[String] = { | ||
| Some(formatDescription(reason, | ||
| sourceLocationUrl = sourceLocationUrl, |
There was a problem hiding this comment.
nitpick: this single named parameter looks odd, can we align it with the rest of the code?
| sourceLocationUrl match { | ||
| case Some(url) => | ||
| // Display a URL to a source location on a CI host. Line numbers are typically referenced with #L anchors. | ||
| s"${url}${l.fileRelativePath}#L${l.line}" |
There was a problem hiding this comment.
Should the user be able to override the anchor with config/env var?
| } yield TestOutcome(name, end - start, res, logs) | ||
| } | ||
|
|
||
| def pure(name: String)(ex: () => Expectations): TestOutcome = { |
There was a problem hiding this comment.
I think it doesn't hurt to keep it either, I've no strong opinion.
| def entries: IO[List[(String, String)]] = IO(List( | ||
| ("WEAVER_SOURCE_URL", | ||
| "https://github.com/typelevel/weaver-test/blob/v0.12.0/") | ||
| )) |
There was a problem hiding this comment.
Let's mention this in the documentation
| G: Concurrent[F], | ||
| C: Clock[F]): F[TestOutcome] = { | ||
| C: Clock[F], | ||
| E: Env[F] |
There was a problem hiding this comment.
I'm not sure it's necessary for the change to impact these layers of the framework. You could centralise the querying of the environment in the formatter (in an unsafe fashion), and apply the github-specific rendering there.
I appreciate that reading the environment does not fall strictly in the pure-FP paradigm that Typelevel ougth to strive for, but the sbt-test-interface that weaver implements forces some compromise onto us. I'm happy to deviate a little bit out of the CE idioms if it minimises the impact / the amount of things the information needs to be threaded through. Additionally, the set of environment variables is pretty much immutable during the lifetime of a program.
3 participants
This resolves #310 .
Source locations in GitHub Actions are displayed as navigable URLs. This can be configured for other CI providers using the
WEAVER_SOURCE_URLenvironment variable.Example
You can find an example in our own GitHub Actions logs
Public API changes
While this is a breaking API change, most users aren't affected by it.
Testconstructor requires an implicitEnv. Most users writing custom test functions will be usingIO, so will have this already.EffectCompatnow requires anEnv. This affects anyone implementing custom effect types.