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 UI test suite #4089

Merged
merged 1 commit into from
Feb 14, 2019
Merged

Improve UI test suite #4089

merged 1 commit into from
Feb 14, 2019

Conversation

vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Jan 16, 2019

Features:

  • updated chromedriver, webdriverio and peer dependencies.
  • broke down AppVeyor test-script into stages (following travis.yml).
  • added appveyor-retry to reduce intermittent failures.
  • reduced log noise (deprecation warnings, reduced verbosity).
  • skipped broken cpp-change-build-config.ui-spec.ts tests.

Signed-off-by: Vincent Fugnitto vincent.fugnitto@ericsson.com

@vince-fugnitto vince-fugnitto changed the title [wip] upgrade UI test dependencies upgrade UI test dependencies Jan 22, 2019
@vince-fugnitto vince-fugnitto force-pushed the vf/fix-ui-tests branch 3 times, most recently from 1c838ef to 5dda0d6 Compare January 23, 2019 13:29
@vince-fugnitto vince-fugnitto force-pushed the vf/fix-ui-tests branch 4 times, most recently from 4f3cef3 to 2bfaba5 Compare January 23, 2019 15:46
@paul-marechal
Copy link
Member

First time in a long time that I see Appveyor green :)

Just clean your commits and should be good to go.

@vince-fugnitto vince-fugnitto force-pushed the vf/fix-ui-tests branch 2 times, most recently from 1db55f1 to af32b23 Compare January 23, 2019 18:10
@vince-fugnitto
Copy link
Member Author

@kittaakos I just wanted your feedback on appveyor.yml since I know you are mainly responsible for it.
I have broken down the job into three chunks (theia, browser, and electron) as well as included the appveyor-retry command to each in hopes of reducing potential intermittent errors. (both are inspired by the recent developments in the travis.yml configuration)

@kittaakos
Copy link
Contributor

I just wanted your feedback on appveyor.yml since I know you are mainly responsible for it.

@vince-fugnitto, unti now... as I see you are just taking it over :)

Once the build is green, it is good to merge. Right now it's broken:

lerna ERR! [chrome #0-4]
3157lerna ERR! [chrome #0-4] 1) theia right panel "before all" hook:
3158lerna ERR! [chrome #0-4] Timeout of 30000ms exceeded. Try to reduce the run time or increase your timeout for test specs (http://webdriver.io/guide/testrunner/timeouts.html); if returning a Promise, ensure it resolves.
3159lerna ERR! [chrome #0-4] Error: Timeout of 30000ms exceeded. Try to reduce the run time or increase your timeout for test specs (http://webdriver.io/guide/testrunner/timeouts.html); if returning a Promise, ensure it resolves.
3160lerna ERR! [chrome #0-4]     at Hook.Runnable._timeoutError (C:\projects\theia\node_modules\wdio-mocha-framework\node_modules\mocha\lib\runnable.js:440:10)
3161lerna ERR! [chrome #0-4]     at Timeout.<anonymous> (C:\projects\theia\node_modules\wdio-mocha-framework\node_modules\mocha\lib\runnable.js:251:24)
3162lerna ERR! [chrome #0-4]     at ontimeout (timers.js:498:11)
3163lerna ERR! [chrome #0-4]     at tryOnTimeout (timers.js:323:5)
3164lerna ERR! [chrome #0-4]     at Timer.listOnTimeout (timers.js:290:5)
3165lerna ERR! [chrome #0-4]
3166lerna ERR! 
3167lerna ERR! info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
3168lerna ERR! 
3169lerna ERR!     at Promise.all.then.arr (C:\projects\theia\node_modules\lerna\node_modules\execa\index.js:236:11)
3170lerna ERR!     at <anonymous>
3171{ Error: Command failed: yarn run test
3172root WARN EditorNavigationContribution.onStart is slow, took: 18904 ms
3173root ERROR Uncaught Exception:  Error: write ECONNRESET
3174root ERROR Error: write ECONNRESET
3175    at Socket._writeGeneric (net.js:768:25)
3176    at Socket._write (net.js:787:8)
3177    at doWrite (_stream_writable.js:396:12)
3178    at writeOrBuffer (_stream_writable.js:382:5)
3179    at Socket.Writable.write (_stream_writable.js:290:11)
3180    at Socket.write (net.js:711:40)
3181    at Sender.sendFrame (C:\projects\theia\node_modules\ws\lib\sender.js:379:20)
3182    at Sender.send (C:\projects\theia\node_modules\ws\lib\sender.js:301:12)
3183    at WebSocket.send (C:\projects\theia\node_modules\ws\lib\websocket.js:336:18)
3184    at WebSocketChannel.doSend (C:\projects\theia\packages\core\lib\node\messaging\messaging-contribution.js:240:24)
3185error Command failed with exit code 1.
3186
3187$ wdio wdio.conf.js
3188

FYI: since you have bumped up the wdio versions it would be nice to see if we can build with Node.js 10. It does not mean we will switch to Node.js 10.

@vince-fugnitto vince-fugnitto changed the title upgrade UI test dependencies Improve UI test suite Jan 23, 2019
@vince-fugnitto vince-fugnitto added ci issues related to CI / tests test issues related to unit and api tests labels Jan 23, 2019
@vince-fugnitto vince-fugnitto force-pushed the vf/fix-ui-tests branch 2 times, most recently from f7f1d7d to d56790f Compare January 23, 2019 23:36
@vince-fugnitto
Copy link
Member Author

@kittaakos the errors are intermittent, and sometimes occur on master, I really need to try and find the root cause. The purpose of the retry is to at least try and limit it, similar to vscode.

From my forked theia (with ci enabled), the tests pass https://ci.appveyor.com/project/vince-fugnitto/theia, I'll re-trigger the build on the main repo.

@vince-fugnitto vince-fugnitto force-pushed the vf/fix-ui-tests branch 5 times, most recently from 4177140 to 9abc375 Compare January 24, 2019 18:53
@vince-fugnitto
Copy link
Member Author

Since are ui test cases are now faster to execute, I thought increasing the timeouts was acceptable.

After this change I noticed travis builds passed consistently each time, and appveyor's intermittent errors (network, timeout, etc.) were more controlled with the additional timeout buffer and retry attempts 😄

I did some research, and it looks like with appveyor's basic plan, performance is not always guaranteed or acceptable, so increasing timeout was a way we can control not exiting our test cases prematurely.

@vince-fugnitto
Copy link
Member Author

@marcdumais-work I guess the next step would be to handle the CQ as you mentioned 😄

@marcdumais-work
Copy link
Contributor

I guess the next step would be to handle the CQ as you mentioned

+1. I am working on it now.

@marcdumais-work
Copy link
Contributor

Umbrella CQ registered, for all the repo's dev-dependencies as of ~1h ago: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=18832 . I believe that it covers all dev-dependencies changed in this PR. Since so far we have not had these dev-dependencies CQs, I would not block this PR to wait for approval, if/when otherwise ready to commit.

@vince-fugnitto vince-fugnitto force-pushed the vf/fix-ui-tests branch 2 times, most recently from 6fcbe0e to 57f10e7 Compare January 25, 2019 18:08
…an up logging)

- Upgraded `chromedriver` to latest compatible version according to chrome version.
- Upgraded peer webdriver dependencies.
- Reduced "noise" in test cases (removed unneeded logging, and deprecation warnings).
- Skip `cpp-change-build-config.ui-spec.ts` test case since it no longer works properly.

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto
Copy link
Member Author

thank you @marechal-p

@vince-fugnitto vince-fugnitto merged commit fa5173d into master Feb 14, 2019
@vince-fugnitto vince-fugnitto deleted the vf/fix-ui-tests branch February 14, 2019 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci issues related to CI / tests test issues related to unit and api tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants