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

prototest: fix early return condition in AssertElementsMatch #17416

Merged
merged 9 commits into from
May 22, 2023

Conversation

rboyer
Copy link
Member

@rboyer rboyer commented May 19, 2023

Description

prototest.AssertElementsMatch has an early return condition that returned early incorrectly when two slices were actually different.

@rboyer rboyer added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test backport/1.13 backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. labels May 19, 2023
@rboyer rboyer self-assigned this May 19, 2023
@@ -63,8 +70,8 @@ func AssertElementsMatch[V any](
}
}

if len(outX) == len(outY) && len(listX) == len(listY) {
return // matches
if len(outX) == len(listX) && len(outY) == len(listY) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bug.

@rboyer rboyer requested a review from analogue May 19, 2023 20:15
@analogue
Copy link
Contributor

analogue commented May 19, 2023

Prefer the improved test in this commit 9fbced6 where Version doesn't have to be singled out for exclusion in the comparison.

rboyer added a commit that referenced this pull request May 19, 2023
rboyer added a commit that referenced this pull request May 19, 2023
rboyer added a commit that referenced this pull request May 19, 2023
Copy link
Contributor

@analogue analogue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@rboyer rboyer added pr/no-backport and removed backport/1.13 backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. labels May 19, 2023
@rboyer
Copy link
Member Author

rboyer commented May 19, 2023

Since the failed tests were all in features recently changed, the backports were all done manually to elide the irrelevant changes.

@rboyer rboyer merged commit e00280e into main May 22, 2023
@rboyer rboyer deleted the fix-prototest-bug branch May 22, 2023 18:49
rboyer added a commit that referenced this pull request May 22, 2023
rboyer added a commit that referenced this pull request May 22, 2023
rboyer added a commit that referenced this pull request May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants