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

feat(ci): add smoke test for domain mutation #6641

Merged

Conversation

anshbansal
Copy link
Collaborator

@anshbansal anshbansal commented Dec 5, 2022

Follow-up #6259 to add tests for domains

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@anshbansal anshbansal requested a review from gabe-lyons December 5, 2022 13:26
@github-actions github-actions bot added the smoke_test Contains changes related to smoke tests label Dec 5, 2022
@github-actions
Copy link

github-actions bot commented Dec 5, 2022

Unit Test Results (build & test)

633 tests  ±0   629 ✔️ ±0   16m 10s ⏱️ +12s
162 suites ±0       4 💤 ±0 
162 files   ±0       0 ±0 

Results for commit d0d4f1c. ± Comparison against base commit 15e9475.

♻️ This comment has been updated with latest results.

cy.get(".ant-checkbox-input").first().click()
cy.get("#continueButton").click()
})
cy.waitTextVisible("Added assets to Domain!")
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's get rid of this one. any of the popover messages, i think we should avoid putting in the tests. we often change the text, and are moving towards getting rid of these!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Domain page does not get updated when assets are added to it. You have to refresh the page before the assets appear. If we refresh the page before assets are added the request might not complete. So this is currently the only way to do this test reliably until we change the domain page to actually refresh the UI when asset addition is complete

Copy link
Contributor

Choose a reason for hiding this comment

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

@anshbansal as a temporary placeholder let's just add a wait then, e.g. 5 seconds

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding wait times will turn this into a flaky test. I had removed all the temp wait times to remove flakiness in these tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No no the caching work has made it instant. Try on demo

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's okay. This is fine for now.

@anshbansal anshbansal added the product PR or Issue related to the DataHub UI/UX label Dec 5, 2022
@anshbansal anshbansal requested a review from jjoyce0510 December 5, 2022 18:01
Copy link
Contributor

@szalai1 szalai1 left a comment

Choose a reason for hiding this comment

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

approving it without context, based on @jjoyce0510 's comment

@anshbansal anshbansal merged commit 626a064 into datahub-project:master Dec 7, 2022
@anshbansal anshbansal deleted the smoke-test-domain-mutation branch December 7, 2022 09:58
cccs-Dustin pushed a commit to CybercentreCanada/datahub that referenced this pull request Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR or Issue related to the DataHub UI/UX smoke_test Contains changes related to smoke tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants