https://pantsbuild.org/ logo
#development
Title
# development
w

witty-crayon-22786

09/16/2021, 8:26 PM
as mentioned, entirely possible that a library will be better. one thing to watch out for is probably libraries that can only consume
junit.xml
produced by
pytest
, for example
a

ancient-vegetable-10556

09/16/2021, 8:28 PM
That makes sense
w

witty-crayon-22786

09/16/2021, 8:31 PM
i know that there was at least one property that was optional in there because one producer of
junit.xml
emitted it, while another didn’t
a

ancient-vegetable-10556

09/16/2021, 8:31 PM
OK.
w

witty-crayon-22786

09/16/2021, 8:32 PM
(which is very FUDy, heh. sorry. i just don’t remember which)
a

ancient-vegetable-10556

09/16/2021, 8:32 PM
😄
Looking through our current Pants 2 code, I see that we store
xml_results
, but it doesn’t look like we do anything to parse it
is that a fair understanding?
w

witty-crayon-22786

09/16/2021, 9:36 PM
correct
it’s attached to workunits as an artifact, so there might be “other consumers” who are parsing it.
a

ancient-vegetable-10556

09/16/2021, 9:37 PM
right
so I take it in the PyTest case, we’re able to determine the results of individual test suites/cases without needing to defer to the XML file?
w

witty-crayon-22786

09/16/2021, 9:41 PM
because of file level deps, we always invoke at the file level, and we use the exit code of pytest for pass/fail
(…works regardless of file-level deps, but yea)
a

ancient-vegetable-10556

09/16/2021, 9:44 PM
OK, staring at the output of a pytest run, I see that, indeed, failures are reported on the file level, and it’s just the console output of pytest that’s showing me the details of the failure
w

witty-crayon-22786

09/16/2021, 9:44 PM
yep. and that’s for the best, because there are lots of fatal errors that result in the test runner not making it far enough to render anything as xml
a

ancient-vegetable-10556

09/16/2021, 9:45 PM
Not having dug into the specifics of JUnit, I’m assuming that we can’t get that useful high-level information out of JUnit, and that’s why we need to consider the XML?
w

witty-crayon-22786

09/16/2021, 9:45 PM
what do you mean?
sorry, re: file level deps: we invoke pytest once per file
a

ancient-vegetable-10556

09/16/2021, 9:46 PM
I know
w

witty-crayon-22786

09/16/2021, 9:46 PM
so we don’t need any high level information from it
but i’m probably not understanding what you are asking
a

ancient-vegetable-10556

09/16/2021, 9:47 PM
The ticket I’m doing is saying that I should parse the JUnit output in order to determine pass/fail status of tests, I’m assuming the ticket I’m working on exists because we can’t get high-level (i.e. “there was a failure”) information without parsing the XML
whereas we can get that from the pytest exit code in the Python case
w

witty-crayon-22786

09/16/2021, 9:47 PM
hm. we … probably don’t need to do that. cc @fast-nail-55400
i could imagine it being runner dependent, but it’s hard to imagine a runner that didn’t exit with non-zero on failure
f

fast-nail-55400

09/16/2021, 9:48 PM
well I already fixed one flaky bug in the existing junit test because it was checking the runner’s output (in addition to the exit code)
that PR is linked in the junit.xml issue
a

ancient-vegetable-10556

09/16/2021, 9:49 PM
(Wow the, GitHub project view is useless)
f

fast-nail-55400

09/16/2021, 9:49 PM
and I modified the junit rule to turn off the fancy output
but in actual use, users might want to see the fancy output
but that makes the test fail
a

ancient-vegetable-10556

09/16/2021, 9:50 PM
So is there a reason why the runner’s exit code is insufficient?
w

witty-crayon-22786

09/16/2021, 9:50 PM
yea, why are we parsing it at all?
f

fast-nail-55400

09/16/2021, 9:50 PM
that is what @bored-art-40741 had for a test so I kept it
it is parsing that the failure that occurred was the expected one
and not some other failure
review what the test is doing and see
w

witty-crayon-22786

09/16/2021, 9:51 PM
i could imagine adding a test that just validates that XML has been captured, but Pants itself doesn’t need to consume it
other consumers might (and it should be attached to the workunit)
(sorry… i hadn’t grokked the underlying motivation for all this)
a

ancient-vegetable-10556

09/16/2021, 9:52 PM
(nor I, hence the questions :D)
f

fast-nail-55400

09/16/2021, 9:52 PM
basically two issues: (1) what to do for Pants users re junit.xml and (2) how to more properly fix the hacky fix in https://github.com/pantsbuild/pants/pull/12909
w

witty-crayon-22786

09/16/2021, 9:53 PM
so assuming we’re capturing it, you could validate that the field is set (and maybe peek inside)
a

ancient-vegetable-10556

09/16/2021, 9:53 PM
My understanding is that junit.xml just needs to be propagated beyond the test, for downstream consumers to deal with; like Pytest does (see @witty-crayon-22786’s above), and the hacky fix is probably fine?
f

fast-nail-55400

09/16/2021, 9:53 PM
https://github.com/pantsbuild/pants/issues/12812 can probably be rephrased from its current formulation
w

witty-crayon-22786

09/16/2021, 9:54 PM
i would remove all parsing of stdout/stderr output from junit
f

fast-nail-55400

09/16/2021, 9:54 PM
the hacky fix is not fine because it hard-codes the test runner to use ASCII output in all cases even ordinary usage by a Pants user
a

ancient-vegetable-10556

09/16/2021, 9:54 PM
OH.
w

witty-crayon-22786

09/16/2021, 9:54 PM
since that’s only in a test, the test could either check the exit code, or peek inside the xml
a

ancient-vegetable-10556

09/16/2021, 9:55 PM
CHecking the exit code won’t suffice, because it’s looking for a specific failure, aiui
f

fast-nail-55400

09/16/2021, 9:55 PM
yes exactly
w

witty-crayon-22786

09/16/2021, 9:55 PM
mm.
f

fast-nail-55400

09/16/2021, 9:55 PM
adding some pass-through configuration to the test runner would let the test of the junit rules pass in the specific config it needs to get ASCII mode
w

witty-crayon-22786

09/16/2021, 9:55 PM
ok! full circle. but non test code doesn’t need to parse. just capture.
f

fast-nail-55400

09/16/2021, 9:55 PM
correct, non-test code does not need to parse
but my hacky flakiness fix is not a good long-term solution so needs something more than the “5 minute” fix I did
w

witty-crayon-22786

09/16/2021, 9:57 PM
iirc, we do something related to
--color
for pytest too. not sure what it is.
a

ancient-vegetable-10556

09/16/2021, 10:15 PM
Oh, I see. So we pass in that that extra flag using
PYTEST_ADDOPTS
, which lets us handle things in a test… so we could probably do the same thing for JUnit to enable output formatting in certain cases?