Skip to content

Commit

Permalink
fix: Use cy.task for health check rather than cy.exec (BREAKING C…
Browse files Browse the repository at this point in the history
…HANGE) (#140)

BREAKING CHANGE:

## The problem

In all of the Percy SDKs we do a thing called a "heath check", which makes sure the Percy agent server is open and ready to accept the DOM we're going to `POST` to it. If the health check fails, we will disable Percy in the SDK so we're not failing your tests due to Percy not running. 

In the Cypress SDK, this was implemented in an interesting way due a limitation with `cy.request`.  The TL:DR of that is we can't `.catch` a failed request, so we needed to use `cy.exec` to health check & gracefully fall out. You can read a little more about that limitation here: cypress-io/cypress#3161

`cy.exec` has it's own issues, which are outlined here: #104 This is a major blocker for _a lot_ of customers. 

## What is this?

This will fix #104 (and hopefully #138).

`cy.task` will always execute within the version of node that is bundled with Cypress, so we no longer have to worry about `$PATH` issues that `cy.exec` faces. We're also no longer running a CLI command for the health check, this new implementation will make a HTTP request from node.

This does mean we need to update the docs to let users know there's an extra step to configure the SDK properly now.

### Upgrading to v2.x

With this change, you will now need to import this new task into your projects plugins (`cypress/plugins/index.js`) file. Without that, the SDK will not work at all. 


```js
/// In cypress/plugins/index.js

let percyHealthCheck = require('@percy/cypress/task')

module.exports = (on, config) => {
  // `on` is used to hook into various events Cypress emits
  // `config` is the resolved Cypress config

  // Make it possible to log things to stdout by calling 'cy.task('log', 'some message to log').
  // Useful for development and debugging.
  on("task", {
    log(message) {
      console.log(message);
      return null;
    }
  });
  on("task", percyHealthCheck);
};
```

## In the future

In an ideal world we would be able to `.catch` on `cy.request` and disable Percy after the first failed `POST` of the DOM. I think in the future we should look into working with Cypress to implement this so we can shrink the integration, make the SDK more reliable, and faster (since we won't make 2 network requests per-snapshot).

## FAQ

#### Why can't you use `fetch` over `cy.request` & `.catch` that?

We need to use `cy.request` since those requests aren't actually executed in the browser and avoid any CORS issues. 

## TODOs

- [x] Update Docs
- [x] How can I guarantee semantic release makes this a major version bump 
- [x] How does one integrate this task into their suite? It's not a default export.. And it runs in node. Maybe it can be apart of the default export
  • Loading branch information
Robdel12 authored Aug 2, 2019
1 parent 19d7b77 commit 40550f7
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 42 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
node_modules/
dist/
cypress/screenshots
14 changes: 8 additions & 6 deletions cypress/plugins/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,19 @@

// This function is called when a project is opened or re-opened (e.g. due to
// the project's config changing)
const percyHealthCheck = require("../../task");

module.exports = (on, config) => {
// `on` is used to hook into various events Cypress emits
// `config` is the resolved Cypress config

// Make it possible to log things to stdout by calling 'cy.task('log', 'some message to log').
// Useful for development and debugging.
on('task', {
log (message) {
console.log(message)
return null
on("task", {
log(message) {
console.log(message);
return null;
}
})
}
});
on("task", percyHealthCheck);
};
60 changes: 25 additions & 35 deletions lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,43 +5,33 @@ declare const Cypress: any
declare const cy: any

Cypress.Commands.add('percySnapshot', (name: string, options: any = {}) => {
const percyAgentClient = new PercyAgent({
handleAgentCommunication: false,
domTransformation: options.domTransformation
})

// Use cy.exec(...) to check if percy agent is running. Ideally this would be
// done using something like cy.request(...), but that's not currently possible,
// for details, see: https://github.com/cypress-io/cypress/issues/3161
const healthcheckCmd = `percy health-check -p ${percyAgentClient.port}`
cy.exec(healthcheckCmd, { failOnNonZeroExit: false }).then(({ stderr }: any) => {
if (stderr) {
// Percy server not available
cy.log('[percy] Percy agent is not running. Skipping snapshots')
cy.log(stderr)

return
}
cy.task('percyHealthCheck').then((percyIsRunning: boolean) => {
if (percyIsRunning) {
const percyAgentClient = new PercyAgent({
handleAgentCommunication: false,
domTransformation: options.domTransformation
})

name = name || cy.state('runnable').fullTitle()
name = name || cy.state('runnable').fullTitle()

cy.document().then((doc: Document) => {
options.document = doc
const domSnapshot = percyAgentClient.snapshot(name, options)
return cy.request({
method: 'POST',
url: `http://localhost:${percyAgentClient.port}/percy/snapshot`,
failOnStatusCode: false,
body: {
name,
url: doc.URL,
enableJavaScript: options.enableJavaScript,
widths: options.widths,
clientInfo: clientInfo(),
environmentInfo: environmentInfo(),
domSnapshot
}
cy.document().then((doc: Document) => {
options.document = doc
const domSnapshot = percyAgentClient.snapshot(name, options)
return cy.request({
method: 'POST',
url: `http://localhost:${percyAgentClient.port}/percy/snapshot`,
failOnStatusCode: false,
body: {
name,
url: doc.URL,
enableJavaScript: options.enableJavaScript,
widths: options.widths,
clientInfo: clientInfo(),
environmentInfo: environmentInfo(),
domSnapshot
}
})
})
})
}
})
})
6 changes: 5 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@
"typescript": "^3.0.3"
},
"dependencies": {
"@percy/agent": "~0"
"@percy/agent": "~0",
"axios": "^0.18.1"
},
"peerDependencies": {
"cypress": "^3"
},
"release": {
"plugins": [
Expand Down
9 changes: 9 additions & 0 deletions task.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const Axios = require("axios");

module.exports = {
percyHealthCheck() {
return Axios.get("http://localhost:5338/percy/healthcheck")
.then(() => true)
.catch(() => false);
}
};

0 comments on commit 40550f7

Please sign in to comment.