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

test: add readiness checks to target page e2es before interacting with visualizations #1751

Merged
merged 2 commits into from
Nov 26, 2019

Conversation

dbjorge
Copy link
Contributor

@dbjorge dbjorge commented Nov 26, 2019

Description of changes

This PR aims to improve an e2e reliability issue that already existed but was exacerbated by #1717 (see motivating failure), where the tabstop tests attempt to interact with the tab stop visualization on the target page before they verify that the target page visualizations are in a ready state. With this PR:

  • The test now waits for the injected shadow root to be present on the target page before attempting to issue "Tab" keystrokes
  • The test's expectations about the resulting tab stop visualizations are now in the form of "expect visualization to reach target state within timeout", rather than "expect visualization state to be X right now"

Cleans up a little page controller method naming nearby as well.

Pull request checklist

  • [n/a] Addresses an existing issue: #0000
  • Ran yarn fastpass
  • [n/a] Added/updated relevant unit test(s) (and ran yarn test)
  • [n/a] Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • PR title AND final merge commit title both start with a semantic tag (fix:, chore:, feat(feature-name):, refactor:). Check workflow guide at: <rootDir>/docs/workflow.md
  • [n/a] (UI changes only) Added screenshots/GIFs to description above
  • [n/a] (UI changes only) Verified usability with NVDA/JAWS

@dbjorge dbjorge requested a review from a team as a code owner November 26, 2019 22:39
popupPage = await browser.newPopupPage(targetPage);
await popupPage.gotoAdhocPanel();
await popupPage.enableToggleByAriaLabel('Tab stops');

await targetPage.bringToFront();
await targetPage.waitForShadowRoot();
Copy link
Contributor

Choose a reason for hiding this comment

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

Love this.

Copy link
Contributor

@Shobhit1 Shobhit1 left a comment

Choose a reason for hiding this comment

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

LGTM

@dbjorge dbjorge merged commit bfe5b52 into microsoft:master Nov 26, 2019
@dbjorge dbjorge deleted the improve-tabstop-e2e-reliability branch November 26, 2019 23:07
dbjorge added a commit to dbjorge/accessibility-insights-web that referenced this pull request Nov 26, 2019
smoralesd pushed a commit that referenced this pull request Dec 2, 2019
This purpose of this PR is to resolve #1591 by replacing our `manifest.json` permissions for `https://*/*", "http://*/*", "file://*/*` with the `activeTab` permission.

To achieve this, it updates the way we track creation and update events for tabs such that we now:
* treat opening the popup view (which gives our extension permissions for the currently-active tab for as long as it stays on the same origin) as "opening" that tab
* treat tab updates/navigation events that change the origin of a tab as having "closed" that tab

This PR only slightly changes the `target-page-controller` implementation, but its tests were a real challenge to follow originally, so the PR completely rewrites `target-page-controller.test.ts` (don't bother comparing them side-by-side, treat it as a new test file for the class).

* refactor: rename Tab.Update and Tab.Change msgs to be distinguishable
* consider origin change events to be like page close events
* prettier format for manifest.json
* Update popup-action-message-creator unit tests per changes
* Update tab store onExistingTabUpdated tests to account for new branch
* Update onNewTabCreated tests to account for new cases
* Add PopupActionCreator tests
* Rewrite target page controller tests (in-progress)
* Refactor simualted browser adapter to separate file, finish test impl
* Fix lint errors
* use isFunction check on setup for addListener mocks
* add tabToContextMap behavior to test corpus
* adjust permission on runtime for e2e to be able to inject js/css into the target page
* Fix merge conflict resolutions
* Fix lint/format issues from merge conflicts
* Add prerequisite "launch popup" step to details view e2e tests
* Update adhoc-panel tests to simulate activeTab permissions
* Fix issue where screenshotOnError would time out and break error stacks if evaluate took too long
* Hacky workaround for adhoc-panel.test timeouts
* Add puppeteer tracing around failing adhoc panel tests
* Avoid issue with page mutation between different parts of clickSelector
* Revert withTracing changes that were extracted to #1751
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants