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

test: install WebdriverIO packages as optional dev dependencies #510

Merged
merged 4 commits into from Aug 17, 2020

Conversation

ctavan
Copy link
Member

@ctavan ctavan commented Aug 14, 2020

No description provided.

We must stay with jest@25.5.4 as long as we want to execute our test in
node@8 (i.e. until node@14 enters LTS).
@ctavan ctavan force-pushed the extract-webdriver-dependencies branch 3 times, most recently from d273ac3 to 2bb5684 Compare August 14, 2020 09:13
@ctavan ctavan changed the title chore: upgrade all devDependencies test: install WebdriverIO packages as optional dev dependencies Aug 14, 2020
@ctavan ctavan force-pushed the extract-webdriver-dependencies branch from 2bb5684 to d82e954 Compare August 14, 2020 09:15
WebdriverIO which we use for browser testing cannot be installed on some
platforms like z/OS.

In order to allow "Canary in the Goldmine"
(https://github.com/nodejs/citgm) to be run on such platforms as well we
need to install WebdriverIO as an optional devDependency. Unfortunately
npm doesn't support this out-of-the-box, so we have to wrap the affected
dependencies in a small local wrapper package.

Fixes #509.
@ctavan ctavan force-pushed the extract-webdriver-dependencies branch from d82e954 to ec68588 Compare August 14, 2020 09:16
@ctavan ctavan marked this pull request as ready for review August 14, 2020 09:23
@ctavan ctavan requested a review from broofa August 14, 2020 09:23
@ctavan
Copy link
Member Author

ctavan commented Aug 14, 2020

@IgorTodorovskiIBM could you check if this branch now lets you run the cigtm test on z/OS?

Something like:

npx citgm https://github.com/uuidjs/uuid/archive/extract-webdriver-dependencies.tar.gz

(or clone the repo & checkout the extract-webdriver-dependencies branch).

@IgorTodorovskiIBM
Copy link

I just tested it on z/OS and your wrapper solution works perfectly. Thank you so much!

@ctavan
Copy link
Member Author

ctavan commented Aug 14, 2020

Cool!

@broofa are you fine with this?

@broofa
Copy link
Member

broofa commented Aug 14, 2020

Hmm... would https://github.com/bcoe/optional-dev-dependency offer a slightly less-complicated solution?

Copy link
Member

@broofa broofa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving since this works and I don't want to block this, but with the caveat that bcoe's module might avoid the need for a whole new subproject folder.

@ctavan
Copy link
Member Author

ctavan commented Aug 14, 2020

Oh, that looks promising. For whatever reason me googling for “optional npm dev dependencies” didn’t find it. I will give it a try!

@ctavan
Copy link
Member Author

ctavan commented Aug 15, 2020

@IgorTodorovskiIBM based on @broofa's feedback I've now chosen a more elegant approach. Would you mind verifying that CITGM still passes on z/OS once again before we merge this?

@IgorTodorovskiIBM
Copy link

Very cool! CITGM still passes! Thanks!

@ctavan ctavan merged commit bc9d4ed into master Aug 17, 2020
@ctavan ctavan deleted the extract-webdriver-dependencies branch August 17, 2020 14:51
@ctavan
Copy link
Member Author

ctavan commented Aug 17, 2020

Thank you for your help in testing this, @IgorTodorovskiIBM!

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