Skip to content

Conversation

@nicktrav
Copy link
Collaborator

@nicktrav nicktrav commented Sep 2, 2025

Fix a testing bug introduced in #131545 where the Contains check arguments were reversed. The error string being searched should precede the string being searched for. The examples from the API docs:

//	require.Contains(t, "Hello World", "World")
//	require.Contains(t, ["Hello", "World"], "World")
//	require.Contains(t, {"Hello": "World"}, "Hello")

Release note: None

Epic: None

Fix a testing bug introduced in cockroachdb#131545 where the `Contains` check
arguments were reversed. The error string being _searched_ should
precede the string being _searched for_. The examples from the API docs:

```
//	require.Contains(t, "Hello World", "World")
//	require.Contains(t, ["Hello", "World"], "World")
//	require.Contains(t, {"Hello": "World"}, "Hello")
```

Release note: None
@nicktrav nicktrav requested a review from a team as a code owner September 2, 2025 20:30
@nicktrav nicktrav requested review from rharding6373 and removed request for a team September 2, 2025 20:30
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

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

😩

Copy link
Collaborator

@rharding6373 rharding6373 left a comment

Choose a reason for hiding this comment

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

Thanks!

@rharding6373
Copy link
Collaborator

Any idea why this test was not failing before, and why is it not failing now that the ordering is fixed?

@andyyang890
Copy link
Collaborator

Any idea why this test was not failing before, and why is it not failing now that the ordering is fixed?

I was wondering the same thing so I ran the test through the debugger and it's because the Error() string is actually identical for both errors

@nicktrav
Copy link
Collaborator Author

nicktrav commented Sep 8, 2025

TFTRs!

bors r+

@nicktrav nicktrav added the backport-all Flags PRs that need to be backported to all supported release branches label Sep 8, 2025
@craig
Copy link
Contributor

craig bot commented Sep 8, 2025

@blathers-crl
Copy link

blathers-crl bot commented Sep 8, 2025

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details.

💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details.

💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details.

💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details.

error creating backport branch refs/heads/blathers/backport-release-25.3-152880: POST https://api.github.com/repos/cockroachdb/cockroach/git/refs: 422 Reference already exists []

Backport to branch release-25.3 failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@nicktrav nicktrav deleted the nickt.fix-changefeed-test branch September 10, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-all Flags PRs that need to be backported to all supported release branches v25.4.0-prerelease

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants