Ticket #10398 (closed: fixed)
Missing destructive test scenarios
Reported by: | Owen Arnold | Owned by: | Harry Jeffery |
---|---|---|---|
Priority: | minor | Milestone: | Release 3.3 |
Component: | Reflectometry | Keywords: | |
Cc: | Blocked By: | #10380 | |
Blocking: | Tester: | Owen Arnold |
Description
The changes made following the http://trac.mantidproject.org/mantid/ticket/10380#comment:7 were nearly correct on the functional side, but insufficient on the side of the testing, and therefore an opportunity has been missed to consider edge cases prior to marking the work as complete. I have passed 10380 because I have manually tested and checked the happy path case provided in the unit test.
The tests that provide the most value are those that have the best ability to uncover errors. Often, these are not the happy path cases. Take for example my manual testing of the options column:
Params='1,1,10', WavelengthMin=1, WavelengthMax=5,u=8
Correctly stops processing and gives warning. Great!
Params='1,1,10', WavelengthMin=1, WavelengthMax=5,
This is allowed?!
Params='1,1,10', WavelengthMin=1, WavelengthMax=5,u
This is also allowed, even though u is not a permissible key value and there is no value for it.
Even if these scenarios above where valid behaviours (and I don't think they should be). The lack of test coverage to demonstrate this implies that they are undesired.
Extend the unit tests, and use TDD with a few more destructive test (read documented) cases. Then fix the the parsing static method to do what you would expect.
Change History
comment:2 Changed 6 years ago by Owen Arnold
- Summary changed from Missing destructive test schenarios to Missing destructive test scenarios
comment:3 Changed 6 years ago by Harry Jeffery
- Status changed from assigned to inprogress
This commit has been deleted.
comment:5 Changed 6 years ago by Harry Jeffery
Refs #10398 Add more aggressive testing
These invalid inputs should throw an exception.
Changeset: 5d9ea12e63ba2cc3944f77e5576312bbe0c3e749
comment:6 Changed 6 years ago by Harry Jeffery
Refs #10398 Update parseKeyValueString to satisfy unit test
Changeset: b94de72e3e6357407de02cc1cf608fd0f021f1dd
comment:7 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
Testing
- Verify tests are passing
- Inspect changes
comment:8 Changed 6 years ago by Owen Arnold
- Status changed from verify to verifying
- Tester set to Owen Arnold
comment:9 Changed 6 years ago by Owen Arnold
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/10398_add_destructive_tests'
Full changeset: 71de4a94152b38249dd7ae8d9db69249a909f4b9
comment:10 Changed 6 years ago by Harry Jeffery
Refs #10398 Add more aggressive testing
These invalid inputs should throw an exception.
Changeset: 7a5d16582025a018923ef082d273357c28be1dcf
comment:11 Changed 6 years ago by Harry Jeffery
Refs #10398 Update parseKeyValueString to satisfy unit test
Changeset: 126a6ef4469d5760b0a6f2d076a682624da3d81e
comment:12 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11240