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

PR #214 does not solve #161 and instead causes the checks to pass even with failing tests. #217

Open
Wilsontomass opened this issue Dec 1, 2022 · 2 comments · May be fixed by #220
Open

Comments

@Wilsontomass
Copy link

Wilsontomass commented Dec 1, 2022

Hi, I dont think PR #214 actually fixes #161, and instead causes a different problem (a subjective one, but still)

Why it doesnt solve #161:
The error in #161 will still fire, as the lines in question are unchanged in #214,

if (results.length === 0) {
  core.setFailed(`No test report files were found`)
  return
}

instead, it changes a different behavior that our repo was relying on:
Our workflow job runs tests and does other things as well as running the test reporter, so even with failing tests, i want that job to pass. Hence, i had:

fail-on-error: 'false'

And then the check that was later created would still "fail", providing us with the information that some tests did indeed fail (and not that the testing suite had failed to run).

#214 has now changed that so both the job and the test report check pass even on failing tests, which is a bit confusing to us at least.
Perhaps the ability to control both would be good?

Elsewise, i will probably split the test reporter out into its own job so that can fail on its own.

@dorny
Copy link
Owner

dorny commented Dec 4, 2022

@Wilsontomass thanks for reporting the issue. I've merged #214 without much thinking about it. I will look into it again.

Wilsontomass added a commit to Wilsontomass/test-reporter that referenced this issue Dec 19, 2022
This also changes how the check is created, failing the check on failing tests.
This resolves dorny#217  and also resolves dorny#161.
@Wilsontomass Wilsontomass linked a pull request Dec 19, 2022 that will close this issue
@gordonbuntting
Copy link

So as far as I can gather, with fail-on-error to true, the reports are not generated if any tests fail. So clearly no good if you want to investigate why a test failed and look at the StdOut etc.

If you set fail-on-error to false then failed tests are reported as success - which is surely not what anyone wants either.

So it seems that this plugin is not currently usable.

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 a pull request may close this issue.

3 participants