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

JUnit output can be rejected by Jenkins's junit test reporter when there are no lint errors #4502

Closed
cjfuller opened this issue May 18, 2023 · 9 comments · Fixed by #4537
Closed
Labels
bug Something isn't working

Comments

@cjfuller
Copy link
Contributor

cjfuller commented May 18, 2023

I'm running ruff . --format junit to lint a codebase on jenkins.

When there are no issues, this produces the following (currently running 0.0.260 on jenkins, but the output is the same locally with 0.0.267):

<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="ruff" tests="0" failures="0" errors="0">
</testsuites>

Jenkins errors on this output with "None of the test reports contained any result," I believe because it doesn't have any <testcase> items in the suite, and I have it set to fail on empty test reports (which I don't want to change because I don't want our CI checks to start silently passing if we introduce a bug in the CI scripts that causes tests not to run).

Semantically it also seems like when lint passes, there should be a passing testcase in here?

I'm able to hack around this if I run this command on Jenkins:

ruff . --format junit | sed -e \
    's/<testsuites name="ruff" tests="0" failures="0" errors="0">/<testsuites name="ruff" tests="1" failures="0" errors="0"><testcase classname="ruff" name="Lint" time="0"><\/testcase>/' \
    > $WORKSPACE/build/test-reports/TEST-ruff.xml

so this isn't a huge problem, but it would be great if ruff included a passing testcase in the output when lint passes.

@charliermarsh charliermarsh added the bug Something isn't working label May 18, 2023
@charliermarsh
Copy link
Member

Taking a look...

@charliermarsh
Copy link
Member

Does the allowEmptyResults option have any impact on this?

@charliermarsh
Copy link
Member

I'm trying to find examples of how other tools handle this. It seems like the ESLint JUnit reporter outputs an empty suite too:

<testsuites name="eslint tests">
</testsuites>

@charliermarsh
Copy link
Member

It looks like this other ESLint JUnit reporter adds an empty test on success, one per file:

<?xml version="1.0" encoding="utf-8"?>
<testsuites>
<testsuite package="org.eslint" time="0" tests="1" errors="0" name="/Users/crmarsh/workspace/test-eslint/baz.js">
<testcase time="0" name="/Users/crmarsh/workspace/test-eslint/baz.js" classname="/Users/crmarsh/workspace/test-eslint/baz" />
</testsuite>
</testsuites>

@charliermarsh
Copy link
Member

I think emitting a passing test for every clean file seems like the clearest thing to do from a semantic perspective. But will it create a lot of noise in the Jenkins output, to have one result per file?

@cjfuller
Copy link
Contributor Author

cjfuller commented May 18, 2023

Does the allowEmptyResults option have any impact on this?

I have not tried (since the job is actively being used right now by other people, and thus I shipped my temporary fix) but my understanding is that turning on allowEmptyResults would fix it, but at the risk of bugs in the jenkins scripts that cause our tests not to run looking like passed runs. Edit: just confirmed in an isolated environment that, yes, allowEmptyResults does cause jenkins to accept ruff's output (though I would still rather not use it).

@cjfuller
Copy link
Contributor Author

But will it create a lot of noise in the Jenkins output, to have one result per file?

This doesn't bother me particularly since for us ruff runs in its own pipeline stage and thus reports separately from everything else, but I could see it mattering to others.

@MichaReiser
Copy link
Member

I think emitting a passing test for every clean file seems like the clearest thing to do from a semantic perspective. But will it create a lot of noise in the Jenkins output, to have one result per file?

This may also not be as straightforward to implement because the JUnit emitter only works on diagnostics, without having access to all files.

@charliermarsh
Copy link
Member

@MichaReiser - Yeah, I think our best bet here is to output a single passing test. I can handle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants