Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maybe fix overmap test (disable geography scaling for tests) #72170

Closed

Conversation

RenechCDDA
Copy link
Member

Summary

None

Purpose of change

overmap_terrain_coverage test fails, a LOT.

Describe the solution

Add external option OVERMAP_GEOGRAPHY_CHANGE which dictates whether these scalars apply at all

Turn the option to off (false) in the TEST_DATA mod, which is (should?) always loaded when the tests are running

Describe alternatives you've considered

master...RenechCDDA:Cataclysm-DDA:maybe_fix_overmap_test

Previous attempt: Added new external option OVERMAPS_BEFORE_GEOGRAPHY_CHANGE, an option that serves as the cutoff where the geography changes kicks in: 10, as was previously expected.

After discussion with the dev team, the PR you're reading was the requested approach.

Testing

Relying mostly on github tests at this point. The test fails so frequently that success in one PR means a lot

Additional context

@RenechCDDA RenechCDDA marked this pull request as draft March 4, 2024 18:46
@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Map / Mapgen Overmap, Mapgen, Map extras, Map display Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Mar 4, 2024
@RenechCDDA RenechCDDA force-pushed the maybe_fix_overmap_test_2 branch from 92dac22 to 57d79da Compare March 4, 2024 19:23
@RenechCDDA RenechCDDA marked this pull request as ready for review March 22, 2024 13:14
@RenechCDDA
Copy link
Member Author

Marked as ready to force it to run all tests

If we still see overmap test fails on this PR, something is wrong and this should probably not be merged.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Mar 22, 2024
@RenechCDDA
Copy link
Member Author

Superseded by #72565

@RenechCDDA RenechCDDA closed this Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant