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

Fix local integration tests #3627

Merged
merged 7 commits into from
Mar 13, 2019
Merged

Fix local integration tests #3627

merged 7 commits into from
Mar 13, 2019

Conversation

nLight
Copy link
Contributor

@nLight nLight commented Feb 27, 2019

This PR fixes the testing env for integration tests
and puts some ducks in the row around naming conventions in the npm scripts.

Testing

Testing OSS:
npm config delete externalplugins - to disable plugins
npm run start:testing
npx cypress open

Testing EE:
npm config set externalplugins <path> - to enable plugins
npm run test:integration:plugins:setup - to copy plugins test. Workaround for cypress
npm run start:testing
npx cypress open

Both should be working

Trade-offs

I couldn't hold myself from cleaning up documentation/scripts

Dependencies

https://github.com/mesosphere/dcos-ui-plugins-private/pull/934

@nLight nLight force-pushed the dima/local-integration-tests branch from 57be6b0 to 37a77de Compare February 27, 2019 18:09
@natmegs
Copy link
Contributor

natmegs commented Feb 27, 2019

I tried to test this but got an error in the cypress UI (EE and OSS):
screen shot 2019-02-27 at 11 47 54 am

GeorgiSTodorov
GeorgiSTodorov previously approved these changes Feb 28, 2019
Copy link
Contributor

@GeorgiSTodorov GeorgiSTodorov left a comment

Choose a reason for hiding this comment

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

Both work for me.
@natmegs , which cypress version are you using?

plugins: [
new EnvironmentPlugin(["NODE_ENV", "DEBUG"]),
Copy link
Contributor

Choose a reason for hiding this comment

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

who is that DEBUG for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fair, removed

// be served, which causes the test to fail. This delays rebuilding the
// application for a very long time when in a testing environment.
const delayTime = 1000 * 60 * 60 * 5;
devServer.watchOptions = {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the benefit over just turning watch off entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, I just restored the config we've had. I believe we still would like to recompile the code as we adapt it. Completely disabling it would mean stoping and restarting the npm run start:testing

@pierrebeitz
Copy link
Contributor

pierrebeitz commented Feb 28, 2019

I have 2 browser-options to test in. If i use Electron everything works just fine. When using Chrome (which is the default for me) i get the same error as @natmegs.

Is this expected? If so: can we add a hint somewhere?

If not expected: i noticed that chrome switches to a route at port 4200 instead of staying on the cypress-port. Maybe that's indicating something :)

dcos-ui 2019-02-28 10-05-10

@GeorgiSTodorov
Copy link
Contributor

Run all specs doesn't work for me either. But individual tests work in Chrome. I use Chrome 71 on Ubuntu 18.04.

@mperrotti
Copy link
Contributor

I'm having the same issue Natalie mentioned.

@nLight
Copy link
Contributor Author

nLight commented Mar 11, 2019

@mperrotti @natmegs does this trick fixes that for you cypress-io/cypress#1872 (comment) ?

@mperrotti
Copy link
Contributor

@nLight - that worked for me :)

mperrotti
mperrotti previously approved these changes Mar 11, 2019
natmegs
natmegs previously approved these changes Mar 11, 2019
@nLight nLight dismissed stale reviews from natmegs and mperrotti via 436b0f7 March 12, 2019 11:43
@nLight nLight force-pushed the dima/local-integration-tests branch from 7ef28fb to 436b0f7 Compare March 12, 2019 11:43
@GeorgiSTodorov
Copy link
Contributor

Build is failing with

npm --unsafe-perm ci— Shell Script1s

+ npm --unsafe-perm ci

npm ERR! @octokit/request not accessible from @octokit/rest


npm ERR! A complete log of this run can be found in:

npm ERR!     /root/.npm/_logs/2019-03-12T12_03_24_859Z-debug.log

script returned exit code 1

@nLight nLight requested a review from mperrotti March 12, 2019 15:51
@nLight nLight merged commit 065c25b into master Mar 13, 2019
@nLight nLight deleted the dima/local-integration-tests branch March 13, 2019 10:38
@mesosphere-ci
Copy link
Collaborator

🎉 This PR is included in version 2.63.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants