fix: hogql tests should run through the query cleaning code to avoid flaps #25900
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
pauldambra
changed the title
pauldambra
commented
Oct 30, 2024
| @@ -31,8 +31,8 @@ | |||
| FROM events LEFT JOIN ( | |||
| SELECT person_static_cohort.person_id AS cohort_person_id, 1 AS matched, person_static_cohort.cohort_id AS cohort_id | |||
| FROM person_static_cohort | |||
| WHERE and(equals(person_static_cohort.team_id, 420), in(person_static_cohort.cohort_id, [6]))) AS __in_cohort ON equals(__in_cohort.cohort_person_id, events.person_id) | |||
There was a problem hiding this comment.
this for example was flapping pretty frequently on cohort id on unrelated changes for me
Twixes
reviewed
Oct 31, 2024
posthog/test/base.py
Outdated
Comment on lines
122
to
123
| query = re.sub(r"(team|cohort)_id(\"?) = \d+", r"\1_id\2 = 2", query) | ||
| query = re.sub(r"\d+ as (team|cohort)_id(\"?)", r"2 as \1_id\2", query) |
There was a problem hiding this comment.
The only thing that feels weird here is that this replacement dummy value 2 look just like normal one 😅 whereas 0000000-0000-0000-0000-000000000000 seems obviously like a test placeholder. Something like 99999 would be more obvious (and 2 has been here before this PR, so I'm introducing scope creep)
There was a problem hiding this comment.
only a tiny bit of creep though
your wish is my command
There was a problem hiding this comment.
🤣 111 files changed 📈
Twixes
approved these changes
Oct 31, 2024
Twixes
approved these changes
Oct 31, 2024
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.
this flapping test has bit me several times today
e1220f0(#25898)and it's confusing to have sql snapshots that don't go through the sql snapshot cleaning code
we aren't using the query cleaning in hogql test sql snapshots, so we periodically flap the snapshots
flapping is slow
so we should avoid it