Windows fixes #101
Windows fixes #101
Conversation
…process is now killed when the tests are complete. Added path fix for Windows, it now works with "/" on Windows in the "test_path" config. Fixed the Process signals to work on Windows.
| @@ -120,7 +120,7 @@ function launchServer() { | |||
| } | |||
|
|
|||
| function launchBrowser(browser, path) { | |||
| var url = 'http://localhost:' + serverPort.toString() + '/' + path; | |||
| var url = 'http://localhost:' + serverPort.toString() + '/' + path.replace(/\\/gi, '/'); | |||
There was a problem hiding this comment.
If all you're replacing is a backslash, you don't need to ignore case, right? Could drop the i flag.
|
Did a test run with this branch on OSX, didn't notice any regressions. Also did a code review. Didn't notice anything serious, mostly style issues. PS: Looks like this replaces the two branches, win_binary and windowsexe. Can delete those. |
|
I have updated the code with the styling and regexp fixes. |
|
Looks good to me. |
|
I tested this on Windows 7 with the following {
"username": "<username>",
"key": "<key>",
"test_framework": "qunit",
"test_path": ["tests/external-repos/underscorejs/test/index.html"],
"browsers": [{
"browser": "firefox",
"browser_version": "15.0",
"device": null,
"os": "OS X",
"os_version": "Snow Leopard"
}]
}It passes on OSX, but on Windows gets stuck forever after outputting "Launching 1 worker(s) for 1 run(s)". @Sanox @bgaillard could one of you look into that? |
|
About five seconds after posting that the runner timed out. Looking at browserstack.com/automate, it seems like the test passed, but it failed to post the result back to the runner. Since the same test passed on OSX, it might be a Windows specific problem. Also tried with "firefox_latest" (dunno why I was using such an old config with FF15), now some tests fail, but the runner still times out eventually. |
|
I have seen the time out issue on Linux as well. But I have found a workaround for browser sessions that is showing a blank page and eventually will time out. The workaround is to join that session via the link "Interactive session" and reload the browser page, then it runs the tests and report the result back. Maybe there is a browserstack issue with OS X that causes it? |
|
I've contacted the BrowserStack team to ask for help. |
|
yep, onto it |
|
i ran underscorejs tests 10 times, and I could reproduce stuck safari in 10th time. Let me dig more. |
|
so I have run some 100 tests on safari8,safari7 post that one test and not able to reproduce at all. I am using 32bit windows machine. Any particular way to reproduce it? |
|
@Sanox @jzaefferer you guys face this quite a lot, or was one off? + I don't think this is related to this pull request per se. what do you suggest? |
|
I just repeated the test I described above, with I also connected to the worker and reloaded the page with developer tools open, but didn't see any errors logged on the console. I also tried to run the QUnit testsuite via its I'm testing with node v0.10.24 (npm 1.3.21) on Windows 7 64-bit, Java SE is 1.8.0_40-b26. @nakula if you're still unable to reproduce any of the issues, it might be good to just merge this PR and do a release, then see if anyone who's actually using Windows still has problems. |
|
I think original issue was: page gets stuck at loading the page and hence the blank screenshot. On reloading the page via Automate dashboard - makes it work. Now, since this happens pretty rarely but in those cases we never get "worker - Acknowledged" in verbose as well. Do you think we can put retry in these cases? reload the url and see if it works? I will make separate issue for implementing it and merge this meanwhile. Now, issue you reported above looks different. Can you re-run in verbose mode once? I have seen that issue when downloaded binary for BrowserStack local is not able to run for some issue. |
Changed to BrowserStackLocal.exe instead of the jar-file.
The tunnel process is now killed when the tests are complete.
Before when using the jar-file the tunnel processes still were alive in the background after the test were completed. Which created problem the next time you ran the tests, your worker then never received the "Acknowledge" state,
Added path fix for Windows, it now works with "/" on Windows in the "test_path" config.
Fixed the Process signals to work on Windows, before you never got to the method "cleanUpAndExit" in cli.js because of how Process.kill works on Windows. It now runs "cleanUpAndExit" before exiting the process as expected.