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

Load all JUnit test reports, not just the first one #2

Closed
tsantalis opened this issue May 19, 2024 · 9 comments
Closed

Load all JUnit test reports, not just the first one #2

tsantalis opened this issue May 19, 2024 · 9 comments
Assignees

Comments

@tsantalis
Copy link

@GaelGirodon

My project has multiple test reports
https://github.com/tsantalis/RefactoringMiner/actions/runs/9143815119/job/25141202337

I added your action in the project's workflow
and it loads only the first one

Load JUnit report '/home/runner/work/RefactoringMiner/RefactoringMiner/build/test-results/test/TEST-org.kohsuke.github.GHRepositoryWrapperTest.xml'
Loaded 1 JUnit report(s)

Is it possible to load all test reports, and save in the json, the total number of passed/failed/skipped tests?

Thank you,
Nikos

@GaelGirodon GaelGirodon self-assigned this May 19, 2024
@GaelGirodon
Copy link
Owner

It's working as intended, the action only loads the first valid report file (closest to the project root directory) for each supported format, but I admit this behavior may not be ideal for all use cases.

I mainly considered Node.js/Go tools that seem to generate a single aggregated report file most of the time, but that's obviously not the case for Maven/Gradle projects.

Parsing all JUnit report files may suit your use case, but not others, e.g. this will break for projects having both aggregated and detailed report files.

In a future release, I'll try to :

  • Find a balanced solution
  • Improve the documentation to be more transparent about this behavior

I could add configuration parameters but I'm trying to make this action as configuration-less as possible to:

  • Make it simple to use
  • Avoid defining parameters that might need to change in the future and might not be backward compatible
  • Avoid having to name things 😅

In the meantime, as a workaround, you can try to:

  • Find a Gradle plugin to generate an aggregated report file, this file will be the one that is loaded if it is located higher than detailed reports in the file hierarchy
  • Generate this file by yourself, e.g. with a simple shell command like find ./path/to/reports/ -name 'TEST-*.xml' -exec cat {} > TEST-results.xml \;

I'm open to ideas & suggestions if you have any! I let this issue opened as this feature request is justified and I'd want to find an appropriate solution.

@tsantalis
Copy link
Author

tsantalis commented May 19, 2024

@GaelGirodon
Thank you for your detailed analysis.

I think it would be nice to have an optional parameter to enable loading all test reports and aggregate the results.
Without specifying this parameter, the default behavior will be the current one.

In Java projects, I would say it is the most common to have multiple test reports.

The change on your side is very simple.
You just need to put the break statement into an if controlled by this parameter.

Actually, I don't see any valid reason to have this break in your code.
Without this break, the code as is, aggregates all test results.
So, the easiest solution is to just delete this break statement.

@GaelGirodon
Copy link
Owner

If I only make the break statement conditional, it will create a badge per test report file and only the last one will be uploaded as all will have the same name, so that's not the desired behavior.

I've done more research about multiple test runners and none of them seem to produce both a single aggregated report and multiple detailed ones. But nearly all of them may produce multiple report files, by default (e.g. Maven Surefire/Failsafe or Gradle), or in case of sharding. Merging multiple report files seems to always require an additional step, that is most of the time unnecessary because reporting tools generally support multiple input files (e.g. GitLab CI, Maven Surefire Report Plugin, the GitHub action you're using, etc.).

So I'm going to modify the action to make it stick to the most common behavior, i.e., load all available test reports. The new version will be released soon, maybe this week, you can preview changes to be released on the main branch.

@tsantalis
Copy link
Author

@GaelGirodon Thank you very much!

GaelGirodon added a commit that referenced this issue May 22, 2024
- Improve JUnit test reports loading
  - Refine file patterns
  - Load all reports instead of only the first one #2
  - Read only top-level `<testsuite>` tags
- Exclude some directories from file search
- Improve documentation
  - Clarify explanations about reports loading
  - Add more examples
- Clean code (quotes, semicolons, imports, tests, ...)
- Update dependencies
  - Update ESLint to v9
GaelGirodon added a commit that referenced this issue May 22, 2024
- Improve JUnit test reports loading
  - Refine file patterns
  - Load all reports instead of only the first one #2
  - Read only top-level `<testsuite>` tags
- Exclude some directories from file search
- Improve documentation
  - Clarify explanations about reports loading
  - Add more examples
- Clean code (quotes, semicolons, tests, ...)
- Update dependencies
  - Update ESLint to v9
GaelGirodon added a commit that referenced this issue May 22, 2024
- Improve JUnit test reports loading
  - Refine file patterns
  - Load all reports instead of only the first one #2
  - Read only top-level `<testsuite>` tags
- Exclude some directories from file search
- Improve documentation
  - Clarify explanations about reports loading
  - Add more examples
- Clean code (quotes, semicolons, tests, ...)
- Update dependencies
  - Update ESLint to v9
@GaelGirodon
Copy link
Owner

The v1.4.0 has just been released 🎉

@tsantalis Could you please check in your project if the issue is resolved?

@tsantalis
Copy link
Author

tsantalis commented May 22, 2024

@GaelGirodon
I just generated the gist with your new release
https://gist.github.com/tsantalis/19fb416d06d1b4d40820e0209540f6c0/revisions

It shows a total of 1728 tests passing.
The correct value is 1718 tests passing + 10 skipped.
https://github.com/tsantalis/RefactoringMiner/actions/runs/9194068100/job/25297450738

I guess the skipped tests are counted as passing.

Anyways, it is far better than before! Thank you for your help.

@GaelGirodon
Copy link
Owner

Thanks for the test!

This is the expected behavior, skipped tests are indeed currently counted as passed tests.

I guess I already have another feature to support for the next release 🙂
Something like this ⬇️?

0 passed
0 passed, 1 skipped
5 passed
4 passed, 1 skipped
4 passed, 1 failed
3 passed, 1 failed, 1 skipped

@GaelGirodon
Copy link
Owner

The v1.5.0 has just been released 🎉

Skipped tests should now be counted and displayed separately, no change is required in the action usage:

tests

Thanks @tsantalis for contributing to the improvement of this action through these feature requests and tests on your project!

@tsantalis
Copy link
Author

@GaelGirodon
I have to thank you for your awesome support.

I confirm it works
image

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

No branches or pull requests

2 participants