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.

Sign up for GitHub

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

Scripts: Remove npm run build from test-e2e default run #13420

Merged
merged 1 commit into from
Jan 29, 2019
Merged

Scripts: Remove npm run build from test-e2e default run #13420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ jobs:
- ./bin/setup-local-env.sh
script:
- $( npm bin )/wp-scripts test-e2e --config=./packages/e2e-tests/jest.config.js --listTests > ~/.jest-e2e-tests
- npm run build
- npm run test-e2e -- --ci --cacheDirectory="$HOME/.jest-cache" --runTestsByPath $( awk 'NR % 2 == 0' < ~/.jest-e2e-tests )

- name: E2E tests (Admin with plugins) (2/2)
Expand All @@ -76,6 +77,7 @@ jobs:
- ./bin/setup-local-env.sh
script:
- $( npm bin )/wp-scripts test-e2e --config=./packages/e2e-tests/jest.config.js --listTests > ~/.jest-e2e-tests
- npm run build
- npm run test-e2e -- --ci --cacheDirectory="$HOME/.jest-cache" --runTestsByPath $( awk 'NR % 2 == 1' < ~/.jest-e2e-tests )

- name: E2E tests (Author without plugins) (1/2)
Expand All @@ -84,6 +86,7 @@ jobs:
- ./bin/setup-local-env.sh
script:
- $( npm bin )/wp-scripts test-e2e --config=./packages/e2e-tests/jest.config.js --listTests > ~/.jest-e2e-tests
- npm run build
- npm run test-e2e -- --ci --cacheDirectory="$HOME/.jest-cache" --runTestsByPath $( awk 'NR % 2 == 0' < ~/.jest-e2e-tests )

- name: E2E tests (Author without plugins) (2/2)
Expand All @@ -92,4 +95,5 @@ jobs:
- ./bin/setup-local-env.sh
script:
- $( npm bin )/wp-scripts test-e2e --config=./packages/e2e-tests/jest.config.js --listTests > ~/.jest-e2e-tests
- npm run build
- npm run test-e2e -- --ci --cacheDirectory="$HOME/.jest-cache" --runTestsByPath $( awk 'NR % 2 == 1' < ~/.jest-e2e-tests )
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@
"publish:dev": "npm run build:packages && lerna publish --npm-tag next",
"publish:prod": "npm run build:packages && lerna publish",
"test": "npm run lint && npm run test-unit",
"pretest-e2e": "concurrently \"./bin/reset-e2e-tests.sh\" \"npm run build\"",
"pretest-e2e": "./bin/reset-e2e-tests.sh",
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this reset do?

Copy link
Contributor

Choose a reason for hiding this comment

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

After looking quickly, this also seems docker specific and it may break some workflows for people running the tests locally. I think we should move it similarly to the travis files.

Copy link
Member Author

@gziolo gziolo Jan 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Not fully sure. This is what I see on the console:

gziolo$ ./bin/reset-e2e-tests.sh 
STATUS: Attempting to connect to WordPress...
STATUS: Resetting test database...
STATUS: Installing WordPress...
STATUS: Ensuring that files can be uploaded...
mode of '/var/www/html/wp-content/uploads' retained as 0767 (rwxrw-rwx)
STATUS: Current WordPress version: 5.0.3...
STATUS: Updating WordPress to the latest version...
STATUS: Checking the site's url...
STATUS: Activating Gutenberg...
STATUS: Installing a dummy favicon...

Copy link
Contributor

Choose a reason for hiding this comment

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

docker-compose $DOCKER_COMPOSE_FILE_OPTIONS run --rm -u 33 $CLI db reset --yes --quiet

It is definitely related to the docker setup.

Copy link
Member

Choose a reason for hiding this comment

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

It's not Travis-specific though; that script is used to reset the test environment so previous test runs don't pollute the results. If you don't reset the tests then when running things locally you might wind up in an unpredictable state.

Copy link
Contributor

Choose a reason for hiding this comment

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

It never happened to me and I never run the test but I agree with you that it's a possibility, the problem is that we can't offer an independent way of doing this reset no matter the WordPress setup used locally. Some use docker, others use vvv and others have local installs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that people run e2e tests against their production websites. It shouldn't be an issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like Travis is super unhappy about removing reset script 🤷‍♂️

"test-e2e": "wp-scripts test-e2e --config packages/e2e-tests/jest.config.js",
"test-e2e:watch": "npm run test-e2e -- --watch",
"test-php": "npm run lint-php && npm run test-unit-php",
Expand Down