Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow output directory for JUnitXmlTestsListener to be configured #6425

Merged
merged 2 commits into from
Apr 12, 2021

Conversation

ashleymercer
Copy link
Contributor

First draft of a PR for this. Some considerations:

  • I've exposed a new testReportsDirectory setting to allow this to be set on a per-configuration basis
  • as a first attempt, I've defaulted this to be target/${config.name}-reports which is a change from the previous behaviour (always defaulted to target/test-reports even for e.g. the IntegrationTest config) - I've added notes about this breaking backwards compatibility but wanted to check this was acceptable? An alternative might be to default to test-reports?
  • I've also (per an existing comment) changed the listener to only be attached to the Test and IntegrationTest configurations directly, rather than at the global level - again, this breaks compatibility for completely custom configs (NB any config which inherits from Test or IntegrationTest will continue to have the listener, which should cover most cases?)

Fixes #2853

@ashleymercer
Copy link
Contributor Author

Okay, binary compat check failed because the listener no longer has an outputDir() property - is it safe to add this to the list of exclusions? Alternatively I can add it back.

class JUnitXmlTestsListener(val outputDir: String, legacyTestReport: Boolean, logger: Logger)
class JUnitXmlTestsListener(val targetDir: File, legacyTestReport: Boolean, logger: Logger)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the current API is using String to pass the absolute path around, then we should keep that behavior to minimize the change in case this listener class is extended by others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I've updated to add (another!) legacy constructor which keeps the old API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(To be clear: the current API accepts outputDir as a String in the constructor, but actually writes to targetDir which is constructed as ${outputDir}/test-reports. I believe the new code follows this behaviour as closely as possible)

@eed3si9n
Copy link
Member

eed3si9n commented Apr 3, 2021

Thanks for working on this!

eed3si9n
eed3si9n previously approved these changes Apr 10, 2021
Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@eed3si9n
Copy link
Member

LGTM. Could you squash the commits and move some of the descriptions to commit message please?

Add `testReportsDirectory` setting to allow output directory for
JUnitXmlTestsListener to be configured.

Add `testReportSettings` which provides defaults values:

- by default this uses the build configuration name as a prefix so
  `target/test-reports` for `Test` config, but `target/it-reports`
  for `IntegrationTest` (previously this was hardcoded to always
  use `target/test-reports`). To override this set e.g.

  `Test / testReportsDirectory := target.value / "my-custom-dir"`

- the `JunitXmlTestsListener` is now only attached to the `Test`
  and `IntegrationTest` configs by default (previously it was added
  to the global configuration object). Any configs which inherit
  from one of these will continue to have the listener attached;
  but completely custom configurations will need to re-add with:

  `project.settings(testReportSettings)`

Fixes sbt#2853
@ashleymercer
Copy link
Contributor Author

LGTM. Could you squash the commits and move some of the descriptions to commit message please?

Done :)

@eed3si9n eed3si9n merged commit 627f72e into sbt:develop Apr 12, 2021
@eed3si9n eed3si9n modified the milestones: 1.5.0, 1.5.1 Apr 26, 2021
mkurz added a commit to sbt/sbt-mocha that referenced this pull request Jan 27, 2024
* They didn't log they results to console anymore since sbt 1.5.1
  -> Caused by sbt/sbt#6425
* Also the total cound of passed/failed tests was not calculated anymore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow junit test-reports folder to be more finely configured
2 participants