Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

PoC cypress #177

Merged
merged 23 commits into from Feb 9, 2021
Merged

PoC cypress #177

merged 23 commits into from Feb 9, 2021

Conversation

jakobw
Copy link
Member

@jakobw jakobw commented Jan 29, 2021

This PR sets up Cypress and adds an initial browser test! \o/

Usage without docker

For running the tests locally npm install installs everything Cypress needs, including Electron. You can then run CYPRESS_BASE_URL=http://your-dev-server:8080 npm run cypress (substituting http://your-dev-server:8080 with the URL where your query builder is running) to run the whole test suite or CYPRESS_BASE_URL=http://your-dev-server:8080 npx cypress open to run the tests interactively.

Usage with docker

Running the tests with docker should be as easy as running docker-compose -f docker-compose.yml -f docker-compose.e2e.yml up e2e. Note that something like 321a1c2 is likely necessary for the Cypress service to reach the web server, but there might be a better way to achieve this.

There are multiple docker images available for Cypress. We chose the included one here, which comes with Cypress preinstalled and multiple web browsers (electron, chrome, firefox). This is definitely not a lightweight option, and it is also worth noting that as it is set up right now, this will use the cypress binary included with the image, not the one from node_modules/. We did this for ease of setup, but please revisit this decision and see if it makes sense.

Problems with the application discovered while creating the tests

Apologies for the noise if these are known.

  1. The biggest problem right now is that the test here is super flaky, which I believe is due to the Property and Item value lookup inputs not being properly debounced, i.e. even if you type real fast the app will make one network request per keystroke. I think this must be fixed in the application code, and is not a problem with the browser test. You can get the tests to pass reliably by adding a delay option to the type() (docs) calls in the tests, but that's silly. I filed T273868.
  2. If you look closely at 2cfc022 you'll notice that it says wdt:undefined on line 49 in the query. The test passes which means that this is actually something that is produced by the query builder, but I fail to reproduce this on the test system. This might be something that got fixed in the meantime.

Problems with the tests

  1. Electron when run in docker seems to cause problems. It hangs indefinitely on the first test without providing useful error output.
  2. The assertion in the browser test currently relies on comparing two url encoded queries. This in itself probably isn't a great idea because test failures won't be very readable, but we also found that there appear to be subtle differences in how browsers do url encoding and the tests currently pass in chrome and electron, but fail in firefox because of that.

for docker-compose command
@bereket-WMDE
Copy link
Contributor

@micgro42
This is a basic implementation of cypress, to get it into github actions.
currently the test only works for chrome. but later (maybe after move to gerrit) should include more browsers.

@jakobw jakobw marked this pull request as ready for review February 8, 2021 11:26
@micgro42
Copy link
Collaborator

micgro42 commented Feb 8, 2021

Strangely, the actual cypress tests have failed. Not sure why GitHub is marking it as successful 🧐

@bereket-WMDE
Copy link
Contributor

@micgro42 I tried it out with cypress io. now when the tests fail, github also fails the job
one problem i'm having is setting the CYPRESS_BASE_URL. since, its not a docker-compose implementation, we need to find a way to find a way to set the required query builder url

@micgro42
Copy link
Collaborator

micgro42 commented Feb 9, 2021

@bereket-WMDE Given how it is done so far, we either need to start the dev server with something like

      - name: Cypress run
        uses: cypress-io/github-action@v2
        with:
          start: npm run serve

see https://github.com/cypress-io/github-action#start-server

or actually add this to the action that deploys to netlify and use that server. We would then probably use the config parameter: https://github.com/cypress-io/github-action#config

@bereket-WMDE bereket-WMDE force-pushed the poc-cypress branch 5 times, most recently from fa602a5 to fcb2d87 Compare February 9, 2021 12:20
@bereket-WMDE
Copy link
Contributor

@bereket-WMDE Given how it is done so far, we either need to start the dev server with something like

      - name: Cypress run
        uses: cypress-io/github-action@v2
        with:
          start: npm run serve

see https://github.com/cypress-io/github-action#start-server

or actually add this to the action that deploys to netlify and use that server. We would then probably use the config parameter: https://github.com/cypress-io/github-action#config

hi @micgro42 the tests fail properly now. yay!
I will address the issues jakob mentioned above in another ticket (when actually implementing the tests). for now, we have a basic github CI cypress integration.

@bereket-WMDE
Copy link
Contributor

@micgro42 updated

Copy link
Collaborator

@micgro42 micgro42 left a comment

Choose a reason for hiding this comment

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

\o/

@sonarcloud
Copy link

sonarcloud bot commented Feb 9, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@bereket-WMDE bereket-WMDE merged commit dec03d0 into master Feb 9, 2021
@bereket-WMDE bereket-WMDE deleted the poc-cypress branch February 9, 2021 15:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
4 participants