[SERVER-30839] buildscripts/combine_reports.py overwrites report.json Created: 25/Aug/17 Updated: 30/Oct/23 Resolved: 05/Oct/17 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Testing Infrastructure |
| Affects Version/s: | None |
| Fix Version/s: | 3.6.0-rc0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Jonathan Abrahams | Assignee: | Robert Guo (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||
| Operating System: | ALL | ||||||||
| Sprint: | TIG 2017-10-02, TIG 2017-10-23 | ||||||||
| Participants: | |||||||||
| Description |
|
The report.json file is overwritten by buildscripts/combine_reports.py, even if there are no input files, i.e., report_reliable.json report_unreliable.json. This behavior prevents tasks which directly create report,json, Jepsen & Powercycle, from producing a report. Additional logic should be added to perform no action if the input files does not exist, but the output does exist. |
| Comments |
| Comment by Githook User [ 05/Oct/17 ] | |||||
|
Author: {'email': 'robert.guo@10gen.com', 'name': 'Robert Guo', 'username': 'guoyr'}Message: | |||||
| Comment by Githook User [ 05/Oct/17 ] | |||||
|
Author: {'email': 'robert.guo@10gen.com', 'name': 'Robert Guo', 'username': 'guoyr'}Message: | |||||
| Comment by Max Hirschhorn [ 05/Oct/17 ] | |||||
|
robert.guo, I'm re-opening this ticket because importing a module shouldn't modify sys.path. The purpose of checking if __name__ == "__main__" and __package__ is None is to check whether the module is being run as the main script.
The idea of moving the import statement out of the if-statement is so that writing python -m buildscripts.resmoke jstests/core/all.js to run resmoke.py works in addition to writing python buildscripts/resmoke.py jstests/core/all.js. | |||||
| Comment by Githook User [ 04/Oct/17 ] | |||||
|
Author: {'email': 'robertguo@me.com', 'name': 'Rob Guo', 'username': 'guoyr'}Message: | |||||
| Comment by Robert Guo (Inactive) [ 25/Sep/17 ] | |||||
|
jonathan.abrahams suggested a better approach. combine_reports should have two modes: one for combining various reports into report.json. The other is as a no-op passthrough, where report.json already exists. This makes the error case straightforward, which is when both or neither the input file and output file exist. If either the input or the output file exist, combine_report should behavior correctly in one of the two modes. I will update the CR accordingly. I think this behavior can be made the default; no new flag is necessary. | |||||
| Comment by Robert Guo (Inactive) [ 29/Aug/17 ] | |||||
|
I think we should error if the output file exists. Otherwise it's easy to end up with the wrong report if we had somehow accidentally created or forgot to delete an report.json. Since the raw report.json from the Evergreen runs is not available, debugging errors could be tricky (e.g. Ditto for if input files do not exist but the output file does. We're running combine_reports.py with --ignore-missing-reports already to ignore missing input files, which is fine because there aren't always unreliable tests. But we should not ignore any existing output files to minimize confusion. |