Simpler Cypress Setup #150

oudeismetis · 2023-01-31T20:34:34Z

What this does

The current Cypress setup is too complicated. This change results in the same behavior with much less code and configuration.

Checklist

Revert to the old simple way of running
Research ways to have Github actions pause until it detects a new deploy has occurred
Implement one of the approaches researched on another project
Confirm that Cypress isn't triggered on Build success, but on Release success
Test what happens when 2 commits are pushed back-to-back
After merge, remove HEROKU_API_KEY and GITHUB_TOKEN from Github and Heroku respectively
After merge, turn the Cypress merge requirement back on.

Deploy Notes

n/a

How to test

You will either need to bootstrap a new project or test this on an existing project (PR author has done the later)

Push a commit and observe that the required Chrome and Firefox steps are yellow.
After the release phase in Heroku, observe that Chrome and Firefox are triggered to run.
Push 2 more commits (spaced ~1min apart)
Observe that when the first release phase finishes, Chrome and Firefox are only triggered on the older of the two commits and that the PR is still blocked for merging.
Observe that when the 2nd release phase finishes, Cypress is finally triggered on the newest commit.

Edge cases

Found only one minor edge case. If you push two commits back-to-back quickly, it's possible that when Cypress runs on the first commit that it will be testing the newer version of code. I think this is a very unlikely scenario to occur and, if it does occur, is unlikely to be noticed or matter to the dev team.

.github/workflows/cypress.yml

oudeismetis · 2023-02-01T20:36:40Z

.github/workflows/cypress.yml

      - run: pipx install pipenv
      - uses: actions/setup-python@v2
        with:
          python-version: '3.10'
-          cache: 'pipenv'
+          # Can't use cache because of https://github.com/actions/cache/issues/319
+          # cache: 'pipenv'


caching doesn't work for deployment_status events sadly. (known bug)

Is this a big deal at the moment ? No right our apps arent that big that it would mean a huge slow down?

It's not really a big deal but it will add time to the test running. So if you push one final commit to the bootstrapper you'll need to wait an extra minute or so.

This does NOT impact the generated projects so definitely not a huge deal in my mind.

ahhh okay good call out!

oudeismetis · 2023-02-01T20:38:26Z

.github/workflows/cypress.yml

@@ -64,7 +37,7 @@ jobs:
          NPM_CONFIG_PRODUCTION: false
        working-directory: ./my_project/client
        run: npm install
-      - name: Run against ${{ needs.Setup.outputs.BASE_URL }}
+      - name: Run against ${{ github.event.deployment_status.environment_url }}


Removed the jwalton/gh-find-current-pr@v1 action dependency as we no longer need to "figure out" what the URL is for the deployment. Heroku gives that to as when it creates the status event.

This is why the Setup step above is now a LOT simpler

oudeismetis · 2023-02-02T14:13:33Z

.github/workflows/heroku_deploy.yml

-            heroku builds:cache:purge --app=${{ needs.Setup.outputs.APP_NAME }} --confirm=${{ needs.Setup.outputs.APP_NAME }}
-            heroku config:set BUILDPACK_RUN="pipenv run cookiecutter . --config-file $config_file --no-input -f" --app=${{ needs.Setup.outputs.APP_NAME }}
+            heroku builds:cache:purge --app=${{ env.APP_NAME }} --confirm=${{ env.APP_NAME }}
+            heroku config:set BUILDPACK_RUN="pipenv run cookiecutter . --config-file $config_file --no-input -f" --app=${{ env.APP_NAME }}
          fi
        env:
          HEROKU_API_KEY: ${{ secrets.HEROKU_API_KEY }}


This Github Action used to do 2 things:
1 - Tell Heroku if it needs to build a Vue app or a React app
2 - Trigger Cypress and cause it to fail early so...(blah blah complicated logic that doesn't exist anymore)

Now all this action does is detect if the dev has touched more React files or more Vue files, and then tell Heroku to change it's build type if a change is needed.

I still don't love that this runs on EVERY commit. I may try to do something smarter, but thats a future PR.

oudeismetis · 2023-02-02T14:14:25Z

{{cookiecutter.project_slug}}/.github/workflows/cypress.yml

@@ -66,4 +34,4 @@ jobs:
          NPM_CONFIG_PRODUCTION: false
          CYPRESS_TEST_USER_EMAIL: "cypress@example.com"
          CYPRESS_TEST_USER_PASS: {{ "${{ secrets.CYPRESS_TEST_USER_PASS }}" }}
-          CYPRESS_baseUrl: {{ "${{ needs.Setup.outputs.BASE_URL }}" }}
+          CYPRESS_baseUrl: {{ "${{ github.event.deployment_status.environment_url }}" }}


All the comments I made above for the bootstrappers cypress.yml are relevant here as well

buuut....since this file is even simpler now, we don't even need a Setup step anymore

oudeismetis · 2023-02-02T14:17:18Z

{{cookiecutter.project_slug}}/scripts/deploy-on-heroku.sh

@@ -3,7 +3,7 @@
 # Deploy {{cookiecutter.project_name}} To heroku
 # NOTE: This script is intended to deploy the app for the first time to heroku
 # if you run it again make and the app is aleady created on heroku make sure to comment lines 6:29 
-APP_NAME={{ cookiecutter.project_slug }}-staging.herokuapp.com
+APP_NAME={{ cookiecutter.project_slug|replace('_', '-') }}-staging.herokuapp.com
 heroku login --interactive
 heroku create $APP_NAME --buildpack heroku/python
 heroku addons:create heroku-postgresql:mini --app $APP_NAME


Heroku requires that our URLs use - instead of _. Our slugs are the opposite (to be Pythonic).
Annoying and an issue you run into if you bootstrap a project from scratch that has a space in it's name.
The fix isn't always obvious. But I think this is now a solved problem.

oudeismetis · 2023-02-02T14:17:49Z

{{cookiecutter.project_slug}}/server/{{cookiecutter.project_slug}}/settings.py

@@ -420,7 +420,7 @@
 # Popular testing framework that allows logging to stdout while running unit tests
 TEST_RUNNER = "django_nose.NoseTestSuiteRunner"

-CORS_ALLOWED_ORIGINS = ["https://{{ cookiecutter.project_slug }}-staging.herokuapp.com", "https://{{ cookiecutter.project_slug }}.herokuapp.com"]
+CORS_ALLOWED_ORIGINS = ["https://{{ cookiecutter.project_slug|replace('_', '-') }}-staging.herokuapp.com", "https://{{ cookiecutter.project_slug|replace('_', '-') }}.herokuapp.com"]


same as above. Heroku requires -

Shouldnt be an issue when we remove heroku default in the BS

Correct, hopefully we can find a way to have separate heroku scripts set values like these for the project based on what was actually created in heroku. Obviously this code is effectively guessing what the URL will be (in a deterministic, but fragile way)

paribaker · 2023-02-02T21:44:56Z

{{cookiecutter.project_slug}}/Procfile

@@ -14,6 +14,6 @@ web: gunicorn {{ cookiecutter.project_slug }}.wsgi --chdir=server --log-file -
 #   3. Up-to-date build numbers shown in app (ideally auto-generated at build-and-deploy time)
 #
 # Comment the line below to disable migrations automatically after a Heroku push.
-release: python server/manage.py makemigrations --noinput && python server/manage.py migrate --noinput && ./scripts/release.sh
+release: python server/manage.py makemigrations --noinput && python server/manage.py migrate --noinput




What about test data? should that not get triggered?

Test data gets run once during review app creation. That happens in a different place than this

tn-spa-bootstrapper/{{cookiecutter.project_slug}}/scripts/db_setup.sh

Line 6 in d2be249

python3 server/manage.py create_test_data

db_setup is what triggers test data to be created.

which is called from the postdeploy Heroku step.

tn-spa-bootstrapper/{{cookiecutter.project_slug}}/app.json

Line 47 in 4197058

"postdeploy": "./scripts/db_setup.sh"

That step only ever gets triggered during review app creation and thus never runs on staging or prod.
Which is why for brand new projects you have to run some of these commands manually once against staging.

paribaker · 2023-02-02T21:45:35Z

{{cookiecutter.project_slug}}/server/{{cookiecutter.project_slug}}/settings.py

@@ -420,7 +420,7 @@
 # Popular testing framework that allows logging to stdout while running unit tests
 TEST_RUNNER = "django_nose.NoseTestSuiteRunner"

-CORS_ALLOWED_ORIGINS = ["https://{{ cookiecutter.project_slug }}-staging.herokuapp.com", "https://{{ cookiecutter.project_slug }}.herokuapp.com"]
+CORS_ALLOWED_ORIGINS = ["https://{{ cookiecutter.project_slug|replace('_', '-') }}-staging.herokuapp.com", "https://{{ cookiecutter.project_slug|replace('_', '-') }}.herokuapp.com"]


Shouldnt be an issue when we remove heroku default in the BS

oudeismetis added the In Progress label Jan 31, 2023

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 January 31, 2023 20:34 Inactive

oudeismetis changed the title ~~Simplier Cypress Setup~~ Simpler Cypress Setup Feb 1, 2023

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 February 1, 2023 17:19 Inactive

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 February 1, 2023 17:50 Inactive

oudeismetis closed this Feb 1, 2023

oudeismetis reopened this Feb 1, 2023

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 February 1, 2023 18:01 Inactive

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 February 1, 2023 18:13 Inactive

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 February 1, 2023 18:23 Inactive

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 February 1, 2023 18:36 Inactive

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 February 1, 2023 18:45 Inactive

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 February 1, 2023 18:55 Inactive

oudeismetis added Ready For Review and removed In Progress labels Feb 1, 2023

oudeismetis commented Feb 1, 2023

View reviewed changes

.github/workflows/cypress.yml Show resolved Hide resolved

oudeismetis commented Feb 1, 2023

View reviewed changes

oudeismetis commented Feb 2, 2023

View reviewed changes

paribaker approved these changes Feb 2, 2023

View reviewed changes

oudeismetis added 7 commits February 2, 2023 20:07

Simplier Cypress Setup

18eb410

trigger Cypress from Heroku deploy status instead

9909161

fix ref to APP_NAME

2d4687e

disable cache

aa2b47c

bad indent

4143739

try a different property

d4e0890

looks like Heroku gives us the URL as well

b12c0e4

welp, cypress just got a LOT simpler

a420569

oudeismetis force-pushed the simpler-cypress-setup branch from d4a9ec6 to a420569 Compare February 3, 2023 01:07

oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-150 February 3, 2023 01:07 Inactive

oudeismetis merged commit edc1627 into main Feb 3, 2023

oudeismetis deleted the simpler-cypress-setup branch February 3, 2023 01:43

oudeismetis mentioned this pull request Feb 23, 2023

Cypress release trigger #99

Merged

12 tasks

This was referenced Mar 10, 2023

Cypress failure for maintenance mode isn't clear #124

Closed

Run Cypress after Heroku Deploy #88

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simpler Cypress Setup #150

Simpler Cypress Setup #150

oudeismetis commented Jan 31, 2023 •

edited

oudeismetis Feb 1, 2023

paribaker Feb 2, 2023

oudeismetis Feb 3, 2023

paribaker Feb 3, 2023

oudeismetis Feb 1, 2023

oudeismetis Feb 2, 2023

oudeismetis Feb 2, 2023

oudeismetis Feb 2, 2023

oudeismetis Feb 2, 2023

oudeismetis Feb 2, 2023

oudeismetis Feb 2, 2023

paribaker Feb 2, 2023

oudeismetis Feb 3, 2023

paribaker Feb 2, 2023

oudeismetis Feb 3, 2023

oudeismetis Feb 3, 2023

oudeismetis Feb 3, 2023

paribaker Feb 2, 2023

Simpler Cypress Setup #150

Simpler Cypress Setup #150

Conversation

oudeismetis commented Jan 31, 2023 • edited

What this does

Checklist

Deploy Notes

How to test

Edge cases

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oudeismetis commented Jan 31, 2023 •

edited