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 js behat #141

Merged
merged 22 commits into from
Feb 2, 2023
Merged

Test js behat #141

merged 22 commits into from
Feb 2, 2023

Conversation

emmacorn
Copy link

@emmacorn emmacorn commented Jan 19, 2023

User story: This is the test branch for JS behat setup.

Description

Checkout the test-js-behat branch
See the file changes in the PR

@emmacorn emmacorn requested a review from byrond January 19, 2023 19:15
@byrond
Copy link
Contributor

byrond commented Jan 25, 2023

Greetings, @agentrickard and @iajon! I thought I'd join the good fight here. I think I finally figured out why Behat is still looking for Selenium at port 8643 even after changing behat.yml. That file gets overwritten when The Build is installed, and it still has the old port. However, I also can't see where we're starting it on port 9515 either, so it might be best to switch back to the old port instead of updating The Build's behat.yml to match.

This has been a longstanding issue, and I dug up this gem from 2018, which I have absolutely no recollection of, but there it is:
#79

It references palantirnet/the-vagrant#62 which is similar to what we're trying to do. Port 4444 was used there, because I think that might be the default for ChromeDriver. However, that requires Java, which I think we're trying to avoid here. I'm going to poke at this a bit and see what I can come up with.

@byrond
Copy link
Contributor

byrond commented Jan 25, 2023

Boom shakalaka! This PR will need to be reviewed, merged, and a new release cut of the-build:
palantirnet/the-build#212

Then, this line will need to be changed to match the new build version:
https://github.com/palantirnet/drupal-skeleton/pull/141/files#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34R35

Also, I haven't tested to see if I broke the local Behat JS functionality.

@byrond
Copy link
Contributor

byrond commented Jan 25, 2023

Also, I think there's an open question of whether this functionality requires the-build. We test this on CircleCI with the-build, and I don't know if there are any cases where we use the skeleton without the-build, but technically it should work. In this case, we will need to update the behat.yml here to match the one installed by the-build. Also, we may need to roll the changes we made to .circleci/config.yaml into the version that gets installed by the-build:
https://github.com/palantirnet/the-build/blob/develop/defaults/install/.circleci/config.yml

Otherwise, new projects created with this skeleton won't run JS tests.

@agentrickard
Copy link
Contributor

@iajon We decided that we should remove the behat.yml file from drupal-skeleton and use the one from the-build.

We do need both circle configs, though.

- run:
name: Run code reviews
command: vendor/bin/phing code-review
name: Install the-build in the project
Copy link
Contributor

Choose a reason for hiding this comment

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

@agentrickard @byrond I tried to commit the newest version of the CircleCI config, but it fails at vendor/bin/phing code-review. The previous version (latest commit) runs vendor/bin/the-build-installer which adds build.xml to the project root. The newest version doesn't run the installer before calling vendor/bin/phing code-review.

Since we are moving behat.yml out of the skeleton, I figure we probably want to fix this in the-build rather than committing build.xml here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@iajon I don't quite follow the logic of this comment, other than to say that yes, the order that things run needs to be settled.

Both projects needs their own circleci config files and they need to match as closely as possible, since the-build-installer will also overwrite the circle config. But it may be that the order in which things run in drupal-skeleton is slightly different.

Only the-build needs build.xml and behat.yml.

Copy link
Contributor

@agentrickard agentrickard left a comment

Choose a reason for hiding this comment

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

I think this is ready once we merge the-build change and make these two updates.

nohup php -S ${CIRCLE_PROJECT_REPONAME}.local:8000 -t $(pwd)/web/ > /tmp/artifacts/phpd.log 2>&1 &
vendor/bin/behat --profile=circleci --suite=default --strict --format=junit --out=/tmp/artifacts
nohup php -S ${CIRCLE_PROJECT_REPONAME}.local:8000 -t $(pwd)/web/ > /tmp/artifacts/phpd.log 2>&1 &
google-chrome --version
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't actually need this line. Its for debugging.

composer.json Outdated
"drupal/core-dev": "^10",
"drupal/drupal-extension": "^5@alpha",
"palantirnet/the-build": "^4@beta"
"palantirnet/the-build": "dev-headless-chrome"
Copy link
Contributor

Choose a reason for hiding this comment

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

We will want to merge the-build change first, and then update this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@agentrickard
Copy link
Contributor

I wonder why this failed?

Copy link
Contributor

@agentrickard agentrickard left a comment

Choose a reason for hiding this comment

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

@iajon We need to start the webserver to run the tests.

@@ -99,8 +99,6 @@ jobs:
- run:
name: Run Behat tests
command: |
nohup php -S ${CIRCLE_PROJECT_REPONAME}.local:8000 -t $(pwd)/web/ > /tmp/artifacts/phpd.log 2>&1 &
Copy link
Contributor

Choose a reason for hiding this comment

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

We need this nohup command to start the web server

@@ -99,8 +99,6 @@ jobs:
- run:
name: Run Behat tests
command: |
nohup php -S ${CIRCLE_PROJECT_REPONAME}.local:8000 -t $(pwd)/web/ > /tmp/artifacts/phpd.log 2>&1 &
google-chrome --version
Copy link
Contributor

Choose a reason for hiding this comment

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

This we can delete

@iajon iajon merged commit 871ccd1 into develop Feb 2, 2023
@iajon iajon deleted the test-js-behat branch February 2, 2023 14:54
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.

4 participants