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

bazel test generate test.xml miss "classname" which cannot pass the junit schema check #16759

Open
doubler opened this issue Nov 14, 2022 · 8 comments
Labels
not stale Issues or PRs that are inactive but not considered stale P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Core Skyframe, bazel query, BEP, options parsing, bazelrc type: feature request

Comments

@doubler
Copy link

doubler commented Nov 14, 2022

Description of the bug:

the bazel test output file test.xml failed to pass the junit validation

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

bazel test //package:target
Check the XML reports are located in bazel-testlogs/package/target/test.xml

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="xxx/smoke_test" tests="1" failures="0" errors="0">
    <testcase name="xxx/smoke_test" status="run" duration="3" time="3"></testcase>
      <system-out>
-----------------------------------------------------------------------------
.
----------------------------------------------------------------------
Ran 9 tests in 0.340s

      </system-out>
    </testsuite>
</testsuites>

🔥 Exception or Error

miss the classname attribute which is required in the junit schema
image

Which operating system are you running Bazel on?

centos

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

Build label: 4.2.3
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Wed Oct 19 08:27:37 2022 (1666168057)
Build timestamp: 1666168057
Build timestamp as int: 1666168057

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

https://cs.opensource.google/bazel/bazel/+/master:tools/test/generate-xml.sh;l=118

Any other information, logs, or outputs that you want to share?

bazelbuild/rules_python#882

@sgowroji sgowroji added type: bug untriaged team-Local-Exec Issues and PRs for the Execution (Local) team labels Nov 14, 2022
@coeuvre coeuvre added team-Core Skyframe, bazel query, BEP, options parsing, bazelrc and removed team-Local-Exec Issues and PRs for the Execution (Local) team labels Nov 22, 2022
@haxorz
Copy link
Contributor

haxorz commented Dec 2, 2022

miss the classname attribute which is required in the junit schema

The Bazel docs say: "The XML schema is based on the JUnit test result schema."

Interpreting this phrasing literally, this is not a bug.

Tbh though I'm not sure if the phrasing perfectly matches the intent of the code. Would you like me to research if the code actually intends to use the JUnit schema, or is my point above sufficient to show this is not a bug and we can close this, or is this actually a FR for Bazel to use the JUnit schema (i.e. use a dummy classname value)?

@haxorz haxorz added the awaiting-user-response Awaiting a response from the author label Dec 2, 2022
@doubler
Copy link
Author

doubler commented Dec 5, 2022

The Bazel docs say: "The XML schema is based on the JUnit test result schema."

I think the doc means bazel test generated XML format should be compatible with the JUnit schema, which require classname.
Many continuous integration(CI) systems follow Junit schema which testing report plugin needs.

Would you like me to research if the code actually intends to use the JUnit schema

Yes, please.

@haxorz haxorz removed the awaiting-user-response Awaiting a response from the author label Dec 5, 2022
@haxorz
Copy link
Contributor

haxorz commented Dec 12, 2022

Okay I've read through the codebase's history and current state. I have a few main observations to share with you:

  • In 2015, b0ba9c9 ("Generate a default dummy XML file when the test runner does not.") added the behavior you referenced in your original post. In the internal code review corresponding to that commit, I see no evidence the author or reviewers considered "required" parts of the jUnit XML schema. I think that's because...
  • (a) The code in generate-xml.sh (used to be in test-setup.sh; got moved there in 0858ae1) fires only if the test action doesn't produce a test.xml file itself. Per the "Role of the test runner" section of the docs the test runner library used by the test is itself supposed to produce a test.xml file, and this is the vastly common case. If the classname thing is really important to you, maybe you could followup in py_test generate test.xml miss "classname" which cannot pass the junit schema check rules_python#882 and figure out what the test runner used there is doing and update it accordingly to produce a XML file.
  • (b) When Bazel internally parses the test.xml file in order to produce a TestCase, it doesn't enforce the official jUnit spec. The combination of this code comment, no attempt to verify the XML meets the jUnit spec (e.g. here), and TestCase.class_name being optional not required 1.

So,

  • (i) I think my earlier interpretation of the docs matches the intent of the docs, namely that the schema of test.xml is not literally the officially jUnit schema, but it's simply inspired by it.
  • (ii) I wouldn't be opposed to changing generate-xml.sh to insert a dummy classname attribute. But that wouldn't help the situation where a test runner produces a test.xml file that doesn't use the attribute.

Footnotes

  1. I know that it's usually a best-practice when designing proto schemas to avoid required but if classname were truly logically required, required would make sense.

@haxorz haxorz added type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed type: bug untriaged labels Dec 12, 2022
@sharmilajesupaul
Copy link

(ii) I wouldn't be opposed to changing generate-xml.sh to insert a dummy classname attribute. But that wouldn't help the situation where a test runner produces a test.xml file that doesn't use the attribute.

@haxorz I think this is a good option. If there are test runners overriding the default xml file I would think the onus would be on the test runner to produce a valid result.

@haxorz
Copy link
Contributor

haxorz commented Dec 14, 2022

Just wanted to clarify:

If there are test runners overriding the default xml

Other way around. The test runner may produce a test.xml file and if it doesn't then Bazel will produce one for it.

That's why in (a) I was saying you could look into making the test runner being used in #882 produce a test.xml file, and then look into making it set the classname attribute.

I would think the onus would be on the test runner to produce a valid result.

Well one of my points thus far has been that "valid" doesn't mean "valid per the junit spec".

But yeah I agree that if you have an issue with a specific test runner you should talk to whoever owns it or fix the problem yourself.

@sharmilajesupaul
Copy link

sharmilajesupaul commented Dec 16, 2022

I think there are cases where we can't always override the xml from inside the test target, like if the test_timeout has been exceeded? As far as I can tell, Bazel will still output it's own xml which is missing classname.

Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 20, 2024
@fmeum
Copy link
Collaborator

fmeum commented Feb 20, 2024

@bazelbuild/triage not stale

@iancha1992 iancha1992 added not stale Issues or PRs that are inactive but not considered stale and removed stale Issues or PRs that are stale (no activity for 30 days) labels Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not stale Issues or PRs that are inactive but not considered stale P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Core Skyframe, bazel query, BEP, options parsing, bazelrc type: feature request
Projects
None yet
Development

No branches or pull requests

7 participants