Repository navigation
Run the shadowed test_strip_relative_path and fix its expectations - #4120
Open
dylanpulver wants to merge 1 commit into
Open
dylanpulver wants to merge 1 commit into
dylanpulver wants to merge 1 commit into
Conversation
test_configs.py defines test_strip_relative_path twice, at line 175 and 296, so the first has never run. The two test different functions, so this renames the second to test_app_strip_relative_path after the function it calls. The revived test then fails: both its expected values carry a leading slash that strip_relative_path does not produce. The live test beside it expects no leading slash anywhere, so these were wrong when written and being shadowed hid it.
|
Contributor
|
Hey @dylanpulver , we're not sure what issue you're encountering that you want this PR to solve. Please open an issue before submitting a PR here. Thanks! |
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.
Targets
devas a fix.tests/unit/test_configs.pydefinestest_strip_relative_pathtwice, at line 175 and line 296. The second replaces the first, so the test at 175 has never run.They are different tests. The one at 175 is the twin of the live
test_get_relative_pathabove it, checking thatapp.strip_relative_pathand the module levelstrip_relative_pathagree with each other and with the expected value. The one at 296 exercisesapp_strip_relative_path. So this renames the second totest_app_strip_relative_path, after the function it calls and matchingtest_invalid_strip_relative_pathbeside it.The rename alone leaves the revived test failing. Both its expected values carry a leading slash that
strip_relative_pathdoes not produce. Withprefix=Noneboth calls returnapp/page2, and withprefix='/app/'both returnpage2. The live test expects no leading slash anywhere, for example("/", "/page-1/sub-page-1", "page-1/sub-page-1"), so these two values were wrong when written and being shadowed from the start hid it. Corrected toapp/page2andpage2.On why this survived: pylint does report
E0102 function already defined line 175, butprivate::lint.pylint-testsruns-d all -e C0410,C0413,W0109, soE0102is not enabled fortests/. With it enabled this file goes from 1 to 0.Verified in a venv on dash 4.5.0, filtering to these tests. Before: 22 collected, 22 passed. Rename only: 24 collected, 2 failed. Both changes: 24 collected, 24 passed. Reverting just the rename returns collection to 22, and corrupting a corrected value fails the test, so both halves carry weight.
black --checkandflake8are clean, andprivate::lint.pylint-testsrates the file 10.00/10.