[SERVER-26523] Replace raise errors.TestFailure with raise errors.ServerFailure in passthrough hooks where appropriate Created: 07/Oct/16 Updated: 20/Jun/17 Resolved: 08/Dec/16 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Testing Infrastructure |
| Affects Version/s: | None |
| Fix Version/s: | 3.4.3, 3.5.1 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Judah Schvimer | Assignee: | Robert Guo (Inactive) |
| Resolution: | Done | Votes: | 0 |
| Labels: | tig-resmoke | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Backwards Compatibility: | Fully Compatible | ||||
| Backport Requested: |
v3.4
|
||||
| Sprint: | TIG 2016-12-12 | ||||
| Participants: | |||||
| Description |
|
None of the current hooks raise errors.ServerFailure when nodes die. Raising errors.ServerFailure rather than errors.TestFailure makes it so the test does not continue on failure. For failures, such as failed teardown, that make further tests worthless, we should raise errors.ServerFailure. |
| Comments |
| Comment by Githook User [ 06/Mar/17 ] |
|
Author: {u'username': u'guoyr', u'name': u'Robert Guo', u'email': u'robert.guo@10gen.com'}Message: (cherry picked from commit e8a3a9266c08bdee7c8772e362cf2b7acd7dab38) |
| Comment by Githook User [ 08/Dec/16 ] |
|
Author: {u'username': u'guoyr', u'name': u'Robert Guo', u'email': u'robert.guo@10gen.com'}Message: |
| Comment by Judah Schvimer [ 07/Dec/16 ] |
|
I do not have a specific failure in mind. I think I agree with Max on this one. While it may reduce the total number of tests we run, it will likely reduce confusion. |
| Comment by Max Hirschhorn [ 07/Dec/16 ] |
Once we've identified a crashing bug in the server and/or a memory leak via ASan/LSan, I don't think there's value in continuing to run tests though. |
| Comment by Robert Guo (Inactive) [ 07/Dec/16 ] |
|
judah.schvimer, I went through all the places where we raise TestFailures in hooks.py. All but the teardown ones are the result of failing to run a command or JS file. I think the reason teardown failures are marked as TestFailures is that they shouldn't prevent additional tests from running. If the fixture crashed during teardown, it should still be able to restart afterwards. If the teardown failed, the fixture should still have been running, meaning the restart had no effect. If the fixture hung, it should be caught by a test timing out. I can see a use-case for marking the initial sync node's failed teardown as a ServerFailure, but I'm not sure if it'd necessarily better than what we currently have. Do you remember if there was a specific failure you had in mind when filing this ticket? |