Skip to content

Conversation

@wmak
Copy link
Member

@wmak wmak commented Oct 18, 2021

  • This also gets the last few tests passing
    • Mocking the snql query too
    • Adding an order to the user_misery test, not sure why this didn't
      flake before, but definitely wasn't passing consistently now
    • We don't allow sorting by issue in the UI, so i assume this is an
      older test. Updated with a sort that does make sense
  • Depends on feat(discover) Add equation support to SnQL #29394

@wmak wmak requested a review from a team October 18, 2021 23:09
@wmak wmak requested a review from a team as a code owner October 18, 2021 23:09
Base automatically changed from wmak/feat/snql-equation-support to master October 19, 2021 19:26
- This also gets the last few tests passing
  - Mocking the snql query too
  - Adding an order to the user_misery test, not sure why this didn't
    flake before, but definitely wasn't passing consistently now
  - We don't allow sorting by issue in the UI, so i assume this is an
    older test. Updated with a sort that does make sense
"transaction",
"user_misery()",
],
"orderby": "user_misery()",
Copy link
Member

Choose a reason for hiding this comment

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

Was this test never flakey somehow? But 👍 to making the result more deterministic.

assert abs(data[1]["user_misery"] - 0.06586) < 0.0001
assert abs(data[2]["user_misery"] - 0.05751) < 0.0001
assert abs(data[1]["user_misery"] - 0.05751) < 0.0001
assert abs(data[2]["user_misery"] - 0.06586) < 0.0001
Copy link
Member

Choose a reason for hiding this comment

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

Thoughts on using pytest.approx going forwards?

Copy link
Member Author

Choose a reason for hiding this comment

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

TIL, we can update in a future PR cc @shruthilayaj (as a btw not saying you're the one who has to do it)

Copy link
Member

Choose a reason for hiding this comment

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

ooh nice, good to know 👍

@wmak wmak merged commit f4c4f63 into master Oct 19, 2021
@wmak wmak deleted the wmak/feat/eventsv2-with-snql branch October 19, 2021 20:44
@github-actions github-actions bot locked and limited conversation to collaborators Nov 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants