Refactor test helpers and clean up random failures #39228
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
Two test cases were failing when run with `--order rand`, because of a failed assumption that the player would not be `in_vehicle` (when in fact other tests spawn vehicles at the same location as `clear_player`, at 60,60,0). This commit adds a `clear_vehicles` test helper that removes all vehicles from the map. The two previously failing test cases now call `clear_vehicles` before making any assumptions about whether the player is in the vehicle. It also changes said assumption to a `REQUIRE_FALSE()` rather than a `CHECK(!)`.
This test was failing on this bit:
npc_test.cpp:440: FAILED:
REQUIRE( rl_dist( g->u.pos(), hostile.pos() ) <= 1 )
with expansion:
14 <= 1
It turns out `place_player` with `tripoint( 10, 10, 0 )` in this test
was locating the player at ( 70, 70, 0 ), and the `spawn_npc` function
(by calling `load_npcs`) was positioning the NPC at ( 80, 80, 0 ). I
have no idea why it is doing this.
But using `place_player` with `tripoint( 0, 0, 0 )` for this test case
allows the player to be positioned at ( 60, 60, 0 ) - again, no idea why
- maybe because that position is used by an earlier test? - anyway, this
lets the NPC be placed at ( 60, 61, 0 ) and thus the test can pass.
Based on running `tests/cata_test -d yes` to see durations: 22.254 s: grenade_lethality 9.236 s: starting_items 7.887 s: starve_test 5.416 s: all_nutrition_starve_test 2.904 s: starve_test_hunger3 1.496 s: unskilled_shooter_accuracy 1.490 s: an unskilled shooter with an inaccurate shotgun 1.167 s: default_overmap_generation_always_succeeds 0.953 s: default_overmap_generation_has_non_mandatory_specials_at_origin
The clear_vehicles() helper function is now defined in player_helpers.(h|cpp)
Move both to map_helpers
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: nonePurpose of change
Several tests were failing when executed with
tests/cata_test --order randwith various seeds, due to stale effects or insufficient cleanup/reset between test cases. This fixes several of them.Describe the solution
item_tname_test.cpp)clear_charactermap_helpers.cppfromvehicle_power_test.cpp:remove_all_vehiclestoclear_vehiclesbuild_test_map, and use the sharedbuild_test_mapfromvehicle_efficiency_test.cppandvehicle_drag_test.cpp[slow]tag to all test cases that took more than 1 second to executetests/cata_test ~[slow]~[.]REQUIRE_FALSEandCHECK_FALSEwhere appropriate (instead of!)Describe alternatives you've considered
Inevitable entropy
Testing
Iterative
tests/cata_test --order rand --rng-seed X, sometimes with~[slow]~[.]to run only the "fast" tests.Additional context
There are bound to be more that still fail with random seeds, but this knocks out 80% of the ones I could find.