Option override support for tests #38051
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ifreund
added
[C++]
ifreund
reviewed
Feb 16, 2020
tests/invlet_test.cpp
Outdated
| @@ -724,25 +725,25 @@ static void merge_invlet_test( player &dummy, inventory_location from ) | |||
|
|
|||
| #define invlet_test_autoletter_off( name, dummy, from, to ) \ | |||
| SECTION( std::string( name ) + " (auto letter off)" ) { \ | |||
| get_options().get_option( "AUTO_INV_ASSIGN" ).setValue( "disabled" ); \ | |||
| override_option _( "AUTO_INV_ASSIGN", "disabled" ); \ | |||
There was a problem hiding this comment.
It took me a second to parse what was happening here due to the fact that we use _( ) for translation. Maybe calling the variable _handle or something would make what's happening a little more obvious?
There was a problem hiding this comment.
Right, that slipped my mind. I renamed them all opt.
This was referenced Feb 16, 2020
jbytheway
force-pushed
the
test_option_override
branch
from
6fb6f57 to
823776f
Compare
This temporarily changes an option value and restores it afterwards.
Replace places where the tests were directly modifying options with uses of the new override_option. This should help to reduce the dependency of the tests on test ordering.
One iteminfo test could fail when the options were configured to metric, so override that option for that test.
jbytheway
force-pushed
the
test_option_override
branch
from
823776f to
a3c5415
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
SUMMARY: Infrastructure "Added override_option class to help write tests which require a particular option to be set"
Purpose of change
Originally inspired by the fact that one of the
iteminfotests fails if the game is set to metric weight display (due to a lbs/kg mismatch).Generalised into a tool for temporarily overriding options in tests, something that several tests need.
Describe the solution
Add
override_option, a new test helper class which provides RAII-style option overriding (set in constructor, reset to old value in destructor).Use this in the places where the tests were already messing with options, and add it also in the particular
iteminfotest that was failing for me.Describe alternatives you've considered
Leaving the old solution where tests changed options and left them changed, but that leads to test ordering dependencies, and makes test failures harder to debug.
Testing
Just unit tests.