[ez] Add --save-xml option to run_test #126690
Conversation
clee2000
changed the title
| @@ -1257,6 +1255,12 @@ def parse_args(): | |||
| action="store_false", | |||
| help="Run tests without translation validation.", | |||
| ) | |||
| parser.add_argument( | |||
There was a problem hiding this comment.
This won't work as expected: common_util parses the cmdline arguments on import and sets variables accordingly.
Some are used by get_report_path which would be broken afterwards:
I'm a bit confused about the run_test function in common_utils and how/where it is used but that would already pass the --save-xml flag on:
pytorch/torch/testing/_internal/common_utils.py
Lines 1058 to 1059 in f9de510
Maybe the 2 functions (in run_test and common_utils) can be merged?
There was a problem hiding this comment.
Renamed to generate xml to prevent clashing, I'll probably get rid of or change the save xml option in common_utils once I figure out what parts of CI it's affecting
There was a problem hiding this comment.
Removing the parents=[common_parser], means that --save-xml is no longer recognized and will throw an error when used. It likely affects much more from that file.
Possibly it is easier just to change https://github.com/pytorch/pytorch/pull/126690/files#diff-e72503c9e3e8766e2d1bacf3fad7b88aa166e0e90a7e103e7df99357a35df8d7R992 to if IS_CI or options.save_xml: and make sure --save-xml is passed to all subprocesses.
|
Closing this, I misread the code so my understanding was wrong |
Move configuration of xml generation from checking for IS_CI to using an argument flag
Remove the parser from common_utils as a parent parser, I don't think any of those options are being used in run_test.py, so if people do want to use them, they can be added using the additional_unittest_args option