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

The Unit Test Results action is failing if there is a test failure #98

Closed
bob-bins opened this issue Mar 17, 2021 · 9 comments · Fixed by #100
Closed

The Unit Test Results action is failing if there is a test failure #98

bob-bins opened this issue Mar 17, 2021 · 9 comments · Fixed by #100

Comments

@bob-bins
Copy link

I'm on v1.9. The README states

Note: This action does not fail if unit tests failed. The action that executed the unit tests should fail on test failure.

But when I run a test containing a failure, the action fails. But if the tests pass, the action passes.

image

image

@EnricoMi
Copy link
Owner

The quoted This action does not fail … refers to the actual publish job (publish_unit_test_results), which is green in both your screenshots. The check run that you are referring to is created by that action to summarize the test results.

To be honest I am not quite sure what is the best behaviour here. One could argue that the Unit Test Results check could represent the outcome of the unit tests and thus should reflect the failure, while the publish_unit_test_result is always green unless there is an exception in processing test results.

On the other hand you could say that the job that ran the unit tests should also fail, thus it is redundant for Unit Test Results to also be in fail state.

Unless there is a good reasoning for the latter point, I think I am shifted towards the first. In that case, I should probably reword that note to be more precise.

@bob-bins
Copy link
Author

The current functionality is rigid and cannot satisfy a use-case where we want certain test failures to be notified on, but not actually cause a failure in the workflow.

What do you think about a boolean parameter for failOnFailedTests defaulting to true instead of forcing a specific behavior?

@EnricoMi
Copy link
Owner

EnricoMi commented Mar 18, 2021

You are right, there are use-cases where you want it to succeed and not pollute your checks. More flexibility over the default behaviour is a good compromise in this case.

Do you want to update your PR or should I sketch it out as it would require README.md and action.yml changes as well?

@bob-bins
Copy link
Author

If you'd prefer to take the task I'd support that. Otherwise I could work on it on the weekend.

@EnricoMi
Copy link
Owner

I will prepare something tomorrow then.

@EnricoMi
Copy link
Owner

Please try the fix by running the action from branch branch-check-fail-behaviour:

uses: EnricoMi/publish-unit-test-result-action@branch-check-fail-behaviour

@bob-bins
Copy link
Author

works great. thanks!

@EnricoMi
Copy link
Owner

Merged to master. This will be released shortly.

@EnricoMi
Copy link
Owner

This has been released. If you are using @v1, it should be picked up on the next run, otherwise upgrade to @v1.10 or @v1.

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.

2 participants