[2.x] test(tags): fetch the created tag by slug rather than by row order - #4963
Merged
Conversation
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.
Surfaced by the reworked CI matrix in #4959, which adds PostgreSQL 18. The job failed on:
Changes proposed in this pull request:
The test creates a tag over the API and then reads it back with:
Taghas no ordering scope, so that is aselectwith noorder by, and which row comes back last is entirely up to the database. When it is not the row just inserted, every assertion after it compares one of the seeded tags instead.It is flaky rather than consistently broken, which is worth being precise about: the PostgreSQL 18 job passed on two earlier runs of #4959 and failed on the third, and in that run the prefixed and unprefixed PostgreSQL 18 jobs disagreed with each other on identical code. PostgreSQL makes it more visible than MySQL for two reasons —
synchronize_seqscansis enabled by default, so a sequential scan may begin part-way through the table to share work with a concurrent scan, and anUPDATEwrites the new tuple version at the end of the heap, moving rows. Neither is specific to version 18, and nothing stops this firing on MySQL.The test means "the tag I just created", so it now says so:
Same class of problem as #4962, which fixed it in the listing code — this is the one remaining instance in a test.
Tag::all()->last()was the only occurrence of the pattern anywhere in the monorepo.This is not a PostgreSQL 18 bug and not a regression. The test has never been sound; it has simply been getting away with it.
Reviewers should focus on:
firstWhereis specific enough. The slug is unique in this fixture and the tag is created by the request under test, so there is exactly one candidate. Ordering byiddescending would also work but would keep the test coupled to insertion order for no benefit.Necessity
Confirmed