Scripts: Remove npm run build from test-e2e default run#13420
Conversation
gziolo
added
[Type] Build Tooling
gziolo
requested review from
a team,
ellatrix,
jorgefilipecosta and
youknowriad
| "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", |
There was a problem hiding this comment.
What does this reset do?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
docker-compose $DOCKER_COMPOSE_FILE_OPTIONS run --rm -u 33 $CLI db reset --yes --quiet
It is definitely related to the docker setup.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I don't think that people run e2e tests against their production websites. It shouldn't be an issue.
There was a problem hiding this comment.
It looks like Travis is super unhappy about removing reset script 🤷♂️
tofumatt
left a comment
tofumatt
left a comment
There was a problem hiding this comment.
What happens if you want to run the tests and the app hasn't been built? Can we at least check for that locally and warn the user about it?
Should we add check if build folder contains subfolders? Would it be enough? |
gziolo
force-pushed
the
update/pretest-e2e
branch
from
6600c41 to
cc6a5a5
Compare
gziolo
force-pushed
the
update/pretest-e2e
branch
from
318b26e to
d3c3636
Compare
|
Travis CI is happy now, I had to leave |
|
You should have left it but only in travis :) |
I'm looking forward to accepting PR you propose :) |
|
GitHub needs a "wow" reaction. 😂😂😂😂😂 |
…rnmobile/372-use-RichText-on-Title-block * 'master' of https://github.com/WordPress/gutenberg: (36 commits) Fixes plural messages POT generation. (#13577) Typo fix (#13595) REST API: Remove oEmbed proxy HTML filtering (#13575) Removed unnecessary className attribute. Fixes #11664 (#11831) Add changelog for RSS block (#13588) Components: Set type=button for TabPanel button elements. (#11944) Update util.js (#13582) Docs: Add accessbility specific page (#13169) Rnmobile/media methods refactor (#13554) chore(release): publish chore(release): publish Plugin: Deprecate gutenberg_get_script_polyfill (#13536) Block API: Parse entity only when valid character reference (#13512) RichText: List: fix indentation (#13563) Plugin: Deprecate window._wpLoadGutenbergEditor (#13547) Plugin: Avoid setting generic "Edit Post" title on load (#13552) Plugin: Populate demo content by default content filters (#13553) RichText: List: Fix getParentIndex (#13562) RichText: List: Fix outdent with children (#13559) Scripts: Remove npm run build from test-e2e default run (#13420) ...
4 participants
Description
This PR remove
npm run buildfrompretest-e2escript which triggered build when runningnpm run test-e2e. This was introduced to ensure that files are built before running e2e tests. However, it is inconsistent with unit tests and causes issues when someone hasnpm run devrunning in the background when developing code. Another issue is that this command was executed even when there were no changes introduced in the codebase since the last run.