Repository navigation
Conversation
The mapset lock and unlock tests only checked the exit status, so a lock command which did nothing would pass. Check that the lock file is created and removed, that locking a locked mapset fails and keeps the lock, that a lock can be forced, and that unlocking a mapset without a lock is not an error. Check the error messages printed when creating a project or a mapset fails. Found with mutation testing, run on the existing tests to look for changes of the code which none of them detect. Written with AI assistance, and the expected values were checked against the behavior of the code.
Test singular unit names and counts other than one for incrementing, test decrementing by a string, including months and years, and test intervals spanning days for compute_datetime_delta, where hours and minutes are computed over days. Found with mutation testing, run on the existing tests to look for changes of the code which none of them detect. Written with AI assistance, and the expected values were checked against the behavior of the code.
Check that MaskManager restores a mask name set before it started, that RegionManager removes the region it saved and that a nested RegionManager makes the outer region active again. Add tests of raster_info, which was only exercised indirectly: the extent and resolution are numbers, and the minimum and maximum are numbers or None for a raster with only nulls. Found with mutation testing, run on the existing tests to look for changes of the code which none of them detect. Written with AI assistance, and the expected values were checked against the behavior of the code.
RegionManager.deactivate() removed the region it saved only when no WIND_OVERRIDE was set before activation. In a nested RegionManager the inner one found the region of the outer one, restored it, and left its own region in the mapset. Remove the saved region in both cases, like MaskManager does with its mask. Found by checking whether the tests detect the removal of the g.remove call, with AI assistance.
RegionManagerEnv saves the value of GRASS_REGION before activation, but no test checked that it is restored when managers are nested. Found with mutation testing, with AI assistance.
This file contains hidden or 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
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.
Description
Strengthen existing tests that executed code but would not fail if it were wrong, and fix a leak in
RegionManagerthat one of the new tests found. Each topic is its own commit:grass.app: the mapset lock and unlock tests were smoke tests checking only the exit status. Now check the lock file, a failed lock on a locked mapset, forced lock removal, and the error messages of failed project and mapset creation.grass.temporal:datetime_mathtests for singular unit names, counts other than one, decrementing by a string, singular units inadjust_datetime_to_granularity, and intervals spanning days incompute_datetime_delta.grass.script:MaskManager,RegionManagerandRegionManagerEnvrestoring the previous state,RegionManagerremoving the region it saved, andraster_infovalue conversions (previously only tested indirectly).grass.scriptfix: a nestedRegionManagerdid not remove its saved region, becauseg.removewas in theelsebranch ofdeactivate()that only runs whenWIND_OVERRIDEwas not set before. It is now removed in both cases, likeMaskManagerdoes.Motivation and context
Mutation testing (one small change to the code at a time, such as deleting a call or swapping an operator, then running the tests that cover that line) showed lines which are executed by tests but where no test fails when the line is changed. The new tests were written to fail on those changes. The nested
RegionManagerleak was found this way.Open question, not changed here: after
mapset lock --force-remove-lockon an already locked mapset, the old lock is removed but no new lock exists, althoughlock_mapset()documents returning the new lock file. The new test does not assert either behavior.How has this been tested?
Mutants killed by the tests before and after, with the same mutants of the same source:
grass/temporal/datetime_math.pygrass/app/cli.pygrass/script/raster.pyThe new tests pass, and the nested
RegionManagertest fails without the fix. Ruff check and format pass for the changed files.Types of changes
Checklist
pre-defined prefixes
of this project.
The tests were written with AI assistance, and the expected values were checked against the behavior of the code.
Generated by Claude Code