bazel icon indicating copy to clipboard operation
bazel copied to clipboard

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

Open doubler opened this issue 3 years ago • 6 comments

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?

https://github.com/bazelbuild/rules_python/issues/882

doubler avatar Nov 14 '22 01:11 doubler

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 avatar Dec 02 '22 17:12 haxorz

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.

doubler avatar Dec 05 '22 01:12 doubler

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

  • In 2015, https://github.com/bazelbuild/bazel/commit/b0ba9c9e7c07e87c1577d2c7ef8517e785528a70 ("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 https://github.com/bazelbuild/bazel/commit/0858ae1f6eb890c1e203a2aa21130ba34ca36a27) 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 https://github.com/bazelbuild/rules_python/issues/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.

[^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 avatar Dec 12 '22 23:12 haxorz

(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.

sharmilajesupaul avatar Dec 14 '22 18:12 sharmilajesupaul

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.

haxorz avatar Dec 14 '22 21:12 haxorz

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.

sharmilajesupaul avatar Dec 16 '22 00:12 sharmilajesupaul

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[bot] avatar Feb 20 '24 01:02 github-actions[bot]

@bazelbuild/triage not stale

fmeum avatar Feb 20 '24 07:02 fmeum