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

Improve test harness and add browser pooling #3871

Merged
merged 169 commits into from
May 12, 2021
Merged

Improve test harness and add browser pooling #3871

merged 169 commits into from
May 12, 2021

Conversation

compulim
Copy link
Contributor

@compulim compulim commented May 2, 2021

Fix #3736.
Fix #3557.
Fix #3842.

Changelog Entry

Added

Fixed

Description

Goals of this PR:

  • Improve DX on test development
    • Converged test behaviors between Chrome and Jest/WebDriver
      • Chrome can use sendKeys to send TAB and other special keys
      • This simplify debugging of tests with race conditions
    • Improved debuggability/runnability in Chrome
    • Auto restart Docker images on local changes
  • Improve test speed
    • Pooling WebDriver session
    • Remove unnecessary Babel/React
  • Improve test reliability
    • Wait for all images loaded before taking snapshot
  • Improve dev scripts on husky, lint-staged and Git pre-commit hook

New "HTML" test harness

Today/yesterday, when we develop some HTML tests that requires special keyboard input (e.g. TAB), it can only be automated via Jest/WebDriver, but not while developing (in pure Chrome). To mitigate the issue, we log to console and ask the developer to do the input manually. This means, tests running in dev mode and Jest/CI could have different behaviors.

We added a new npm run browser command to start a Chrome that can be remotely controlled by a corresponding WebDriver client. It can send special keyboard inputs, emulated pointer input, and take screenshots.

Please download a matching version of ChromeDriver.exe and put it under the project root. If you are on WSL, you will need chmod +x ChromeDriver.exe to enable execute permission.

  • Run npm run browser to start a browser session for HTML tests
    • Using this browser to load __tests__/*.html will use WebDriver to run tests on them
    • View the screenshot from console for easy debugging
  • Run npm run docker to start a Docker Compose
    • This is only needed for Jest
    • Any files changed, will rebuild and restart of the compose automatically

Separate test harness and test page object model

Today/yesterday, test harness and page object model are mingled together. Clearer responsibilities on packages will increase DX.

It was packages/testharness, and now into 2 packages: packages/test/harness and packages/test/page-object-model.

  • Test harness is the infrastructure to run tests, it use WebDriver behind the scene
  • Test page object model is the object model for controlling Web Chat, no knowledge of WebDriver is needed
  • Both test harness and page object models does not use regenerator-runtime, give a much better error stack

Test code changed

  • body element will have a class named jest, if the page is running under Jest (or CI)
    • Can be used to hide elements for test development
  • await pageConditions.uiConnected(), instead of await host.wait(conditions.uiConnected(), 15000)
  • Use run(async () => {}), instead of IIFE with host.done() or host.error()
  • No Babel or React is loaded by default, should be quicker
  • Standardized all xxxStabilized code by leveraging the new pageConditions.stabilized()
    • See scrollStabilized() or the advanced scrollToBottomCompleted() for usage

Browser session pooling

Today/yesterday, browser sessions are disposed immediately after tests. It takes about 1 second to launch a new browser session. With ~240 HTML tests, it took 4 extra minutes.

This feature reduce that 4 minutes to (240 / 5) * 1s ~= 48 * 1s = 48 seconds (-192 seconds).

Added pooling by a WebDriver proxy, given capabilities are unchanged.

  • Browser sessions are now pooled with max 5 runs and 3 minutes (the last 30 seconds are not acquirable)
  • Saved about 1 second per test run that can reuse sessions, or about 3 minutes
  • Increase memory from 2 GB to 2.5 GB for each session for stability
  • Jest only parallelize test suites, but not tests inside each suite
  • Few things to reset:
    1. Navigate to about:blank
    2. Move mouse to (0, 0)
    3. Reset window size to 360x640

Design

The "WebDriver session pooling server" proxy will intercepts new session and delete session requests.

We move window resizing from capabilities to runtime. This means all tests no longer have unique capabilities, this enable simple session pooling.

Every instance in the pool will survive 5 runs, or 3 minutes maximum. We assume each session will take up to 30 seconds. That means, after hitting 2:30 minutes, the instance will no longer to acquire. If it was acquired, it will be continue available until release.

Specific Changes

(Other than specified above)

  • Rename Dockerfile-xxx to xxx.dockerfile for VSCode syntax highlighting
  • Updated npm run docker to pick up changes and restart images automatically
  • Separated test harness and test page object model packages
  • Update some snapshots
    • Button click in the new test harness will also move the mouse (to mimic a real mouse behavior)
      • Thus, the snapshot will show hover effect on the button
    • Some test reliabilities
  • Cleaner HTML test files
    • Less one-time imports
  • Don't load React/Babel in test harness, HTML file need to specify them if needed
    • Not every test need JSX, not loading React/Babel will speed up tests
  • No more pre-build for testharness, means, npm install or npm run bootstrap will be faster
  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation (Will do in another PR)

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

Copy link
Contributor Author

@compulim compulim left a comment

Choose a reason for hiding this comment

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

Commented.

@compulim
Copy link
Contributor Author

3 things to discuss offline. I have resolved all comments. Please let me know if I overlooked anything. 😉

Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

LGTM - don't see a need to wait for our 1:1 to approve :)

@corinagum
Copy link
Contributor

corinagum commented May 11, 2021

image
Looks like the failing test is an unintended bug:

image
(non-interactive card should not be focusable)

I think this test is incorrectly named - behavior looks correct to me, considering there are inputs within the card.

@compulim compulim merged commit 3c216e1 into main May 12, 2021
@compulim compulim deleted the qol-r14-2 branch May 12, 2021 03:00
@compulim compulim mentioned this pull request May 12, 2021
11 tasks
compulim added a commit that referenced this pull request Jun 22, 2021
* Update husky and lint-staged

* Add debug

* Only test transcript navigation tests

* Add message

* Disable ESLint

* Run transcript.navigation tests only

* Add more tests

* Add messages

* Fix syntax error

* Update logic

* Add new message

* Add message

* Update reporter

* Add message

* Log even if error

* Clean up logs

* Update splice

* Fix scrollStabilized

* Fix timeout

* Add testharness2

* Move transcript.navigation.escapeKey

* Move transcript.activityGrouping

* Move transcript.activityGrouping

* Clean up

* Move transcript.navigation.pageUpDown

* Move transcript.navigation.adaptiveCard.focusInput

* Move transcript.navigation.*

* Move some accessibility tests

* Move more accessibility tests

* Move all accessibility tests

* Move all activities.* tests

* Move activityGrouping.* tests

* Move adaptiveCards.* and autoScroll.*

* Move avatar.* tests

* Move cardAction.* tests

* Move carousel.* tests

* Move chatAdapter.* tests

* Move conversationStartProperties.* tests

* Move deprecated.* and directLine.* tests

* Move all focusManagement.* tests

* Move heroCard.* tests

* Move hooks.* tests

* Move locale.* and markdown.* tests

* Move middleware.* tests

* Move newMessageButton.* tests

* Move offlineUI.* tests

* Move replyToId.* tests

* Move sendBox.* tests

* Move sendFiles.* tests

* Move speechRecognition.* tests

* Move styleOptions.* tests

* Move suggestedActions.* tests

* Move timestamp.* tests

* Move toast.* tests

* Move transcript.* tests

* Move use*.* tests

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Fix Docker script

* Improve toolchain

* Default to no-watch

* Fix tests

* Fix tests

* Fix tests

* Fix tests and clean up

* Fix build

* Add WD pool for Jest

* Update housekeeping

* Clean up

* Add jestserver

* Update housekeep

* Touchup

* Consistent window size

* Gracefully shutdown Jest server

* Gracefully shutdown Jest server

* Clean up

* Clean up instance after release

* Ignore get logs error

* Clean up

* Housekeep asynchronously

* Rename

* Dump logs before quit

* Use global-agent

* Improve test reliability

* Take fullscreen shot

* Move session preparation code

* Fix attachment

* Improve error stack

* Speed up

* Fix test

* Fix test

* Prepare session on reuse

* Update snapshot

* More __operation__

* Fix code coverage

* Increase shm_size

* Converge

* Fix tests

* Use stabilized

* Use stabilized

* Wait for focus

* Remove one screenshot

* Rename folders

* Build test harness

* Clean up test harness

* Renames

* Clean up

* Fix build

* Fix test

* Remove unneeded plugin

* Fix test

* Fix tests

* Fix tests

* Fix tests

* Fix test

* Add comments

* Add entry

* Add eslint

* Enable eslint

* Fix ESLint

* Fix ESLint

* Update husky

* Update lint-staged

* Update precommit

* Update precommit

* Update comment

* Remove unnecessary line

* Screenshot after fail

* Wait for image load before snapshot

* Wait for activity to be focused

* Attach last screenshot

* Wait for activities

* Wait for capacity

* Add screenshot location

* Fix error message

* Fix filename

* Add entry

* Clean up

* Fix tests

* Add comment

* Clean up

* Clean up

* Fix test reliability

* Add tolerance

* Update entry

* Improve test reliability

* Remove loop

* Update entry

* Improve test reliability

* Add all attachments

* Apply PR suggestions

* Apply suggestions from code review

Co-authored-by: Corina <[email protected]>

* Add comment

* Namespacing accessibility.delayActivity

* Update tofu

* Fix test

* Fix test

* Fix test

* Test reliability

* Apply PR suggestions

* Remove unneeded marshal

* Update message

Co-authored-by: Corina <[email protected]>
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.

husky-run not found Improve tests speeds by reusing tabs Clean up tests
2 participants