Fix busted tests due to a bug in PeriodicValueConverter from Flexbox changes #153
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
Some places was using just .Periodic() without any labels specified which triggered the debug assertion that the label count was 2 or 4. Because all the value converter stuff is static, the exception raised during the static constructor crashed the XUnit test runner process and masked the fact these tests were failing because they was never actually being run. Fixed by treating the case of no labels the same as 4 for backwards compatibility.
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.
The flexbox PR changed PeriodicValueConverter to work with 2 values instead of only 4, but what was missed was some places were using just
.Periodic()without any labels specified which triggered the debug assertion on the label count being 2 or 4.Because all the value converter stuff is static, the exception raised during the static constructor crashed the XUnit test runner process and hid the fact these tests were failing because they were never actually being run.
This fixes this by treating the case of no labels the same as 4 for backwards compatibility. Confirmed that all tests are now passing with none marked as being skipped.