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

chore(test): add macro performance tests for merge #280

Closed
wants to merge 1 commit into from

Conversation

kwonoj
Copy link
Member

@kwonoj kwonoj commented Sep 8, 2015

Try to close #257.

After adding this test case I'm observing failures on my machine at the end of perf test using Windows 10, Chrome 45.0.2454.85 m (64-bit). To verify it's occurring on other configuration created clean machine runnning Ubuntu 12.02 LTS 45.0.2454.85 (64-bit) and test was exited without error. Since test case itself ran successfully I suspect this is somewhat related with protractor issue (maybe this? angular/protractor#2001) but do not have concrete clue. While trying to troubleshoot issue, creating PR to ask if anyone experiences similar cases.

github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\webdriver.js:377
throw new Error('This driver instance does not have a valid session ID ' +
^
Error: This driver instance does not have a valid session ID (did you call WebDriver.quit()?) and may no longer be used.
at checkHasNotQuit (Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\webdriver.js:377:13)
at Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\webdriver.js:350:5
at [object Object].webdriver.promise.ControlFlow.runInNewFrame_ (Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\promise.js:1654:20)
at [object Object].webdriver.promise.ControlFlow.runEventLoop_ (Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\promise.js:1518:8)
at [object Object].wrapper as _onTimeout
at Timer.listOnTimeout (timers.js:110:15)
==== async task ====
WebDriver.quit()
at [object Object].webdriver.WebDriver.schedule (Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\webdriver.js:345:15)
at [object Object].webdriver.WebDriver.quit (Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\webdriver.js:418:21)
at Z:\github\RxJS\node_modules\protractor\lib\driverProviders\driverProvider.js:59:14
at Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\goog\base.js:1582:15
at [object Object].webdriver.promise.ControlFlow.runInNewFrame_ (Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\promise.js:1654:20)
at notify (Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\promise.js:465:12)
at [object Object].then (Z:\github\RxJS\node_modules\protractor\node_modules\selenium-webdriver\lib\webdriver\promise.js:522:7)
at [object Object].DriverProvider.quitDriver (Z:\github\RxJS\node_modules\protractor\lib\driverProviders\driverProvider.js:57:23)
at Array.map (native)
[launcher] Process exited with error code 1

@kwonoj
Copy link
Member Author

kwonoj commented Sep 9, 2015

Tried some more scenarios to dig this and came to suspect it's related with protractor. Long story in short, trying with latest version of protractor (2.2.0) makes this problem goes away. Unfortunately do not have answer why current version (1.7.0) fails though. (Tested on systems I have, Windows 10+Chrome 45 x 2 + Ubuntu+Chrome 45 x 1).

@benlesh
Copy link
Member

benlesh commented Sep 9, 2015

@kwonoj ... according to @jeffbcross we should have a new version of benchpress shortly (releases are tied to Angular releases). Perhaps that will also solve this problem?

@kwonoj
Copy link
Member Author

kwonoj commented Sep 9, 2015

@Blesh I have not updated version of benchpress but only updated protractor and it seems solves this specific problem. If updating version of benchpress & protractor can be aligned together, I think it'll solve this issue. Does benchpress has specific version dependency to protractor?

@kwonoj
Copy link
Member Author

kwonoj commented Sep 9, 2015

+if benchpress update comes shortly, my suggestion is below :

  1. Mark macro perf test PR as blocked for now
  2. Once benchpress is ready, create PR for bump up version of benchpress & protractor both and merge it
  3. Once test regression is verified, merge pending PR if exists

What do you think?

@benlesh
Copy link
Member

benlesh commented Sep 9, 2015

Considering all macro perf tests are broken for me at the moment, and in most cases we're adding tests where nothing existed before, I think we're okay to merge any good looking macro perf test as-is and create issues for corrections if need be once benchpress is sorted out.

@kwonoj
Copy link
Member Author

kwonoj commented Sep 9, 2015

I'm fine with allowing merge and take further action after benchpress update. Suggestion above is just based on personal preferences :)

@benlesh
Copy link
Member

benlesh commented Sep 9, 2015

Merged with 26661c8

Thanks @kwonoj!

@benlesh benlesh closed this Sep 9, 2015
@kwonoj kwonoj deleted the chore-macro-perf-test-merge branch September 9, 2015 19:50
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore(test): add macro performance test for merge
2 participants