penthouse icon indicating copy to clipboard operation
penthouse copied to clipboard

Support for windows development

Open Saturate opened this issue 7 years ago • 3 comments

This package works great on windows, however as it is right now it's not testable on windows. As seen in #234 this can course some issues when trying to contribute with a windows machine.

For windows support, the first step is to make all the tests support windows, and then enable a appveyor build, to keep it tested and supported.

This issues is tracking this progress and discussion. I intend to take a look at this at some point.

Saturate avatar Feb 25 '18 15:02 Saturate

Great initiative. 👍

For most tests it is hopefully just cleaning up path and process usage to make sure it works on windows as well, but the node module tests (for running multiple penthouse calls in parallel) that currently in a very hacky way rely on process path matching will need to completely be re-written. I only started thinking about it that a more sane way to test these patterns (was the same browser used) would be with test spies, i.e. just count how many times the critical methods/functions were called.

Note in general that I have been touching quite a lot of the core code recently just to play around with running more processes in parallel to increase performance. To update the tests it should not be needed to look at the core code, but headsup just in case.

pocketjoso avatar Feb 25 '18 15:02 pocketjoso

So far I have had luck with __dirname, this seems to replace the intention of process.env.PWD.

+return 'file://' + path.join(__dirname, 'static-server', file)
-return 'file://' + path.join(process.env.PWD, 'test', 'static-server', file)

This alone made all tests pass on windows, so I guess supporting it is really simple. Should I just create a PR with this, and maybe a AppVeyor config?

Edit: All the tests in appveyor , so not done yet.

Saturate avatar Feb 25 '18 16:02 Saturate

Hmm I had issues with __dirname when I switched to the Jest taskrunner, that's the reason I switched. But I didn't look into it deeply so it should be possible to revert and fix the problem I had (relative paths making tests fail) in another way.

pocketjoso avatar Feb 25 '18 16:02 pocketjoso