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

WIP: Windows and PhantomJS 2.0 #505

Merged
merged 1 commit into from Mar 22, 2015

Conversation

johanneswuerbach
Copy link
Member

Chocolatey is installing PhantomJS now inside %PATH%

@johanneswuerbach johanneswuerbach changed the title Fixed windows CI WIP: Windows and PhantomJS 2.0 Feb 20, 2015
@johanneswuerbach
Copy link
Member Author

The issue is that chocolatey is using a shim now to exec phantomjs. After testem received all test results from PhantomJS, it tries to kill the process, but only has the PID of the shim, which already exited.

I don't have an idea how to workaround this atm. 😞

Using plain PhantomJS 2.0 on Windows works as expected.

//cc @stefanpenner

@stefanpenner
Copy link
Contributor

ahh, can we pull in someone from the chocolatey team?

@johanneswuerbach
Copy link
Member Author

Maybe @ferventcoder or @Iristyle? Is there a way to keep the shim running and kill the tree?

@ferventcoder
Copy link

Help me understand what you want to do. It sounds like this tool should kill phantomjs.exe (the actual, not the shim). The shim should be blocking until phantomjs shuts down.

@ferventcoder
Copy link

I'm guessing you kill the shim and you want to kill the entire tree. I think you already stated that.

As you can see here the shim is blocking. It won't exit on its own until the phantomjs.exe (actual) exits.
image

@johanneswuerbach
Copy link
Member Author

Currently I'm sending SIGTERM to the process with the pid I spawned (which I guess is the shim). This seems to kill the shim, but not phantomjs. When I'm installing phantomjs manually or use 1.9.8 this works as expected.

I'll look into other ways to kill the tree on windows, but other test runners are using the approach and it works well a cross platforms and browsers.

@ferventcoder
Copy link

I wonder if we can enhance shims that receive a SIGTERM to destroy the tree? That seems like a much better option.

@ferventcoder
Copy link

I created https://github.com/chocolatey/shimgen/issues/11 to handle this.

@johanneswuerbach
Copy link
Member Author

That sounds great. Here is a small example script to reproduce the issue:

var cp = require('child_process');
var p = cp.spawn('phantomjs', ['--webdriver=8910'], {}); // The wd arg just keeps phantom listening
p.on('close', function() {
  console.log('Should be called.')
});
setTimeout(function() {
  p.kill();
}, 2000);

@ferventcoder
Copy link

Thanks. I will have something out after a bit.

@ferventcoder
Copy link

So I can't find a nice way to handle SIGKILL but SIGTERM is going to be in Chocolatey 0.9.9 (you will need to regenerate your shims). It will be out in rc8 (due out this weekend).

johanneswuerbach added a commit that referenced this pull request Mar 22, 2015
@johanneswuerbach johanneswuerbach merged commit b2fed62 into testem:master Mar 22, 2015
@johanneswuerbach johanneswuerbach deleted the fix-appveyor branch March 22, 2015 18:15
@ferventcoder
Copy link

I take it you are good to go? :)

@johanneswuerbach
Copy link
Member Author

Yes, I had to switch from process.kill to executing taskkill /T on Windows as of #520 (npm is creating a cmd file when installing phantomjs), which also solves this issue.

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.

None yet

3 participants