Tests: give seed on default tests and fix execnet error #3520
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
github-actions
bot
added
the
waiting-on: peer-review
Exempt-Medic
added
the
is: enhancement
NewSoupVi
reviewed
Jun 12, 2024
ScipioWright
added
is: bug/fix
Silvris
changed the title
| excluded = self.multiworld.worlds[self.player].options.exclude_locations.value | ||
| state = self.multiworld.get_all_state(False) | ||
| for location in self.multiworld.get_locations(): | ||
| if location.name not in excluded: | ||
| with self.subTest("Location should be reached", location=location): | ||
| with self.subTest("Location should be reached", location=location.name): |
There was a problem hiding this comment.
does this keep failing the serialization dumping? i thought we had a proper repr for location, but if we don't i think that's probably the better fix for this
There was a problem hiding this comment.
This is the cause of the serialization function failing within tests.
There was a problem hiding this comment.
Looking at it, Location does have a _repr_ that calls _str_. We could use str(location) instead, but the end result is the same.
NewSoupVi
approved these changes
Jun 22, 2024
qwint
pushed a commit
to qwint/Archipelago
that referenced
this pull request
Jun 24, 2024
…W#3520) * output seed of default tests * test execnet fix * try failing with interpolated string * Update bases.py * try without tryexcept * Update bases.py * Update bases.py * remove fake exception * fix indent * actually fix the execnet issue --------- Co-authored-by: NewSoupVi <[email protected]>
GameWyrm
pushed a commit
to GameWyrm/Archipelago-GW
that referenced
this pull request
Jul 4, 2024
…W#3520) * output seed of default tests * test execnet fix * try failing with interpolated string * Update bases.py * try without tryexcept * Update bases.py * Update bases.py * remove fake exception * fix indent * actually fix the execnet issue --------- Co-authored-by: NewSoupVi <[email protected]>
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.
What is this fixing or adding?
https://github.com/ArchipelagoMW/Archipelago/actions/runs/9488885076/job/26148887892
A test failure that gives effectively no information about the failure.
With these changes, we catch the exception being thrown such that the seed passed intosubTestwill get printed into the logs, allowing for reproduction of the error locally. While this is a stopgap solution, I couldn't find much information about the cause of the error itself.The issue is that we pass
locationintoself.subTest, when really all we want there islocation.name. This PR instead fixes this issue alongside providing seed on failing tests.How was this tested?
https://github.com/Silvris/Archipelago/actions/runs/9489978465/job/26152426677https://github.com/Silvris/Archipelago/actions/runs/9533282014/job/26276353017
Manually, by forcefully failing the test within the for loop of all_state_can_reach_everything.
Unittests were also ran locally.