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

feat: Ability to snapshot a directory of webpages. Usage: percy snapshot directory/ #137

Merged
merged 33 commits into from
Apr 26, 2019

Conversation

maprules1000
Copy link
Contributor

@maprules1000 maprules1000 commented Apr 11, 2019

Description

This adds a service to support taking snapshots of static sites. Now, instead of having to install a higher level SDK and then add the Percy snapshot command to tests, if you have a pre-compiled, server-ready directory of files percy-agent can snapshot them with ease.

A large chunk of this PR is a simple test site that was built independent of any static site generators. It contains non-final-season Game of Thrones spoilers. You've had 2 years to catch up.

package.json Outdated
@@ -128,6 +130,7 @@
"test": "npm run build-client && PERCY_TOKEN=abc mocha --forbid-only \"test/**/*.test.ts\" --exclude \"test/percy-agent-client/**/*.test.ts\" --exclude \"test/integration/**/*\"",
"test-client": "mkdir -p dist-test/ && npm run build-client-test && testem ci --file ./test/percy-agent-client/testem.js",
"test-integration": "npm run build-client && node ./bin/run exec -- mocha test/integration/**/*.test.ts",
"test-snapshot-command": "./bin/run snapshot test/integration/testStaticSite -b /dummy-base-url -i '(red-keep)' -c '\\.(html)$'",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test site has been built with this exact command and arguments in mind. Changing this command could require changing the test site.

But json files don't have comments so there's no great place to document that.

return this.options
}

async _buildPageUrls() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where most of the magic happens. This command walks the local directory and looks at each file in the directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using globby instead of walk makes this function even simpler

Copy link
Contributor

Choose a reason for hiding this comment

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

so much win!

@maprules1000 maprules1000 marked this pull request as ready for review April 23, 2019 16:06
Copy link
Contributor

@cadeParade cadeParade left a comment

Choose a reason for hiding this comment

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

Come for the GoT spoilers, stay for the typos

const ignoreFilesRegex = flags['ignore-files'] as string
const snapshotFilesRegex = flags['snapshot-files'] as string
const rawIgnoreGlob = flags['ignore-files'] as string
const rawSnapshotGlob = flags['snapshot-files'] as string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these globs will come in as strings and need to be converted to an array of strings to work with the globby package

src/commands/snapshot.ts Outdated Show resolved Hide resolved
src/commands/snapshot.ts Outdated Show resolved Hide resolved
src/commands/snapshot.ts Outdated Show resolved Hide resolved
src/commands/snapshot.ts Outdated Show resolved Hide resolved
src/commands/snapshot.ts Outdated Show resolved Hide resolved
src/commands/snapshot.ts Outdated Show resolved Hide resolved
if (this.server) { await this.server.close() }
}

_buildLocalUrl() {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


})

describe('#_buildPageUrls without the ignore flag set', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this test covers the case when the ignore flag is an empty array


const snapshotGlobs = rawSnapshotGlob.split(',')

const ignoreGlobs = rawIgnoreGlob ? rawIgnoreGlob.split(',') : []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Split on an empty string results in an array of an empty string. Globby thinks an empty string means all files, and we def do not want to ignore all files by default. If the raw string is empty then the empty string is falsey and we pass an empty array to the static snapshot service, otherwise we pass an array of the ignore globs.

@djones djones changed the title feat: static snapshot service feat: Added the ability to snapshot a directory of webpages. Usage: percy snapshot directory/ Apr 26, 2019
@djones djones changed the title feat: Added the ability to snapshot a directory of webpages. Usage: percy snapshot directory/ feat: Ability to snapshot a directory of webpages. Usage: percy snapshot directory/ Apr 26, 2019
Copy link
Contributor

@djones djones left a comment

Choose a reason for hiding this comment

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

🍍 LGTM

@maprules1000 maprules1000 merged commit 20daabb into master Apr 26, 2019
@maprules1000 maprules1000 deleted the map/static-site-service branch April 26, 2019 23:37
djones pushed a commit that referenced this pull request Apr 26, 2019
# [0.4.0](v0.3.1...v0.4.0) (2019-04-26)

### Features

* Ability to snapshot a directory of webpages. Usage: `percy snapshot directory/` ([#137](#137)) ([20daabb](20daabb))
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