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

[MLv2] Track drill-thru unit test coverage #36601

Closed
wants to merge 1 commit into from

Conversation

bshepherdson
Copy link
Contributor

It's not checked anywhere yet, but this is a first cut at some
bookkeeping helpers we can use to ensure that all the (many!)
permutations of drill-thrus are getting properly checked by the unit
tests.

Copy link
Contributor Author

bshepherdson commented Dec 8, 2023

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@bshepherdson bshepherdson requested review from camsaul and a team December 8, 2023 20:49
@bshepherdson bshepherdson self-assigned this Dec 8, 2023
@bshepherdson bshepherdson added the no-backport Do not backport this PR to any branch label Dec 8, 2023
@metabase-bot metabase-bot bot added the .Team/QueryProcessor :hammer_and_wrench: label Dec 8, 2023
@bshepherdson bshepherdson added this to the 0.49 milestone Dec 8, 2023
@@ -107,6 +107,7 @@
(deftest ^:parallel table-view-available-drill-thrus-headers-pk-test
(testing "column headers: click on"
(testing "primary key - column filter (default: Is), sort, summarize (distinct only)"
(lib.drill-thru.tu/coverage-available! 0 0 :header :pk)
Copy link
Member

Choose a reason for hiding this comment

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

Would this be better if we just made a wrapper around available-drill-thrus that looked at the query shape and context shape and recorded this stuff, and then made sure lib.drill-thru.tu/test-available-drill-thrus and other stuff in the drill thru test namespaces used it? Same for lib/drill-thru.

Copy link
Member

@camsaul camsaul left a comment

Choose a reason for hiding this comment

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

PR approved if you want to check this in

It's not checked anywhere yet, but this is a first cut at some
bookkeeping helpers we can use to ensure that all the (many!)
permutations of drill-thrus are getting properly checked by the unit
tests.
Copy link

replay-io bot commented Jan 26, 2024

StatusComplete ↗︎
Commite298c4a
Results
1 Failed
  • should allow non-user to unsubscribe from subscription
      AssertionError: Timed out retrying after 4000ms: Expected to find content: 'You've unsubscribed non-user@example.com from the "Orders in a dashboard" alert.' but never did.
          at Context.eval (http://localhost:4000/__cypress/tests?p=e2e/test/scenarios/sharing/subscriptions.cy.spec.js:45369:17)
⚠️ 1 Flaky
  • should allow users to provide internal urls
2253 Passed

@bshepherdson
Copy link
Contributor Author

I'm closing this PR, as the drill-thru test coverage has moved in a different direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport Do not backport this PR to any branch .Team/QueryProcessor :hammer_and_wrench:
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants