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

Fix flaky tracking integration tests #3561

Closed
nathaniel-may opened this issue Jul 12, 2021 · 3 comments · Fixed by #3604
Closed

Fix flaky tracking integration tests #3561

nathaniel-may opened this issue Jul 12, 2021 · 3 comments · Fixed by #3604
Assignees
Labels
tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality

Comments

@nathaniel-may
Copy link
Contributor

Postgres integration test suite 033 matches expected and actual tracking events. However, the experimental parser fires its tracking event non-deterministically. Commits 7a32129, and c43c5fa were supposed to fix this, yet circleci still sees inconsistent failures.

This set of tests may not actually be all that useful as is and we may want to rework how we test events going forward anyway.

@nathaniel-may nathaniel-may added the tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality label Jul 12, 2021
@nathaniel-may nathaniel-may self-assigned this Jul 12, 2021
@leahwicz
Copy link
Contributor

If this test is flaky and we don't have an obvious way to fix it, can we comment it out for now just to unblock CI?

@kwigley
Copy link
Contributor

kwigley commented Jul 21, 2021

We can definitely do that! But, that will only address tests that directly test tracking events. There are still intermittent failures in other tests (example). This is because some tests bootstrap a (sometimes incomplete) project config and this doesn't always initialize the global current user. Since there are tracking events being sent non-deterministically when parsing model files, some tests are hitting this code path that expect an initialized global user! I tried to address this in #3604

@leahwicz
Copy link
Contributor

Thanks @kwigley! Just unblocking ourselves here for now will be super helpful and then we can dive deep on what is actually going on with these tests and make them more stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants