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

Internal: Convert ghostjs to puppeteer #182

Merged
merged 21 commits into from May 17, 2018
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions .buildkite/pipeline.yml
Expand Up @@ -56,14 +56,10 @@ steps:
- BUILDKITE_COMMIT
- BUILDKITE_PROJECT_SLUG
- CODECOV_TOKEN
- name: ":ghost: integration %n"
command: ./run_integration_tests
- name: ":ghost: Integration Tests"
command: yarn run test:integration
artifact_paths: 'screenshots/*'
timeout_in_minutes: 30
parallelism: 3
env:
- BUILDKITE_PARALLEL_JOB
- BUILDKITE_PARALLEL_JOB_COUNT
agents:
queue: elastic
plugins:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@

### Patch
* Internal: Add code coverage to PRs (#185)
* Internal: Internal: Convert ghostjs to puppeteer (#182)
* Internal: Update Jest and use multi-project runner (#158)

</details>
Expand Down
17 changes: 10 additions & 7 deletions Dockerfile
Expand Up @@ -9,20 +9,23 @@ ENV DISPLAY :99
COPY test/xvfb_init /etc/init.d/xvfb
COPY test/xvfb_daemon_run /usr/bin/xvfb-daemon-run

RUN curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - && \
echo "deb http://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list && \
apt-get update -yy -qq && \
apt-get install yarn -yy -qq && \
apt-get install xvfb firefox-esr -yy -qq && \
chmod a+x /etc/init.d/xvfb /usr/bin/xvfb-daemon-run

COPY yarn.lock package.json ./

RUN mkdir -p docs test packages/gestalt
COPY packages/gestalt/package.json ./packages/gestalt/
COPY docs/package.json ./docs/
COPY test/package.json ./test/

RUN apt-get update \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you want this RUN command to be high so it's not invalidated by changes to the package.json etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chrislloyd indeed - moved this one (and the other RUN command) back up.

&& apt-get install -yq libgconf-2-4 \
&& apt-get install -y wget --no-install-recommends \
&& wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add - \
&& sh -c 'echo "deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main" >> /etc/apt/sources.list.d/google.list' \
&& apt-get update \
&& apt-get install -y google-chrome-unstable --no-install-recommends \
&& rm -rf /var/lib/apt/lists/* \
&& npm i puppeteer@1.4.0
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a dependency of the tests - any reason why you're doing this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chrislloyd was a trail / error of combining multiple docker commands. Was able to get rid of it.


RUN yarn install --pure-lockfile --ignore-scripts

COPY . ./
1 change: 1 addition & 0 deletions dangerfile.js
@@ -1,4 +1,5 @@
// @flow
/* eslint-disable no-console */
import { danger, markdown, warn } from 'danger';
import { execSync } from 'child_process';
import { readFileSync } from 'fs';
Expand Down
7 changes: 5 additions & 2 deletions package.json
Expand Up @@ -39,8 +39,10 @@
"gzip-size": "^4.1.0",
"identity-obj-proxy": "^3.0.0",
"jest": "^22.4.3",
"jest-puppeteer": "^3.0.1",
"node-fetch": "^2.1.2",
"prettier": "1.10.2",
"puppeteer": "^1.4.0",
"react-test-renderer": "^16.2.0",
"stylelint": "^8.0.0",
"stylelint-config-prettier": "^2.1.0",
Expand All @@ -53,7 +55,7 @@
"lint:css": "stylelint \"packages/**/*.css\"",
"start": "concurrently \"cd docs && yarn start\" \"cd packages/gestalt && yarn watch\"",
"test": "jest",
"ghost": "BUILDKITE_PARALLEL_JOB_COUNT=${BUILDKITE_PARALLEL_JOB_COUNT} BUILDKITE_PARALLEL_JOB=${BUILDKITE_PARALLEL_JOB} ./run_integration_tests",
"test:integration": "./run_integration_tests",
"format": "prettier --write \"{.babelrc,.eslintrc,**/*.{js,css,md}}\""
},
"jest": {
Expand Down Expand Up @@ -149,5 +151,6 @@
],
"value-no-vendor-prefix": true
}
}
},
"dependencies": {}
}
26 changes: 0 additions & 26 deletions packages/gestalt/src/Icon/__integration__/Icon.a11y.ghost.js

This file was deleted.

@@ -0,0 +1,32 @@
/* global page */
import assert from 'assert';

describe('Icon > a11y', () => {
beforeAll(async () => {
await page.addScriptTag({ path: require.resolve('axe-core') });
});
it('Loads at least 5 icons on the test page', async () => {
await page.goto('http://localhost:3001/A11y?component=icon');

const svgIcons = await page.$$('svg');
assert.ok(svgIcons.length >= 5);
});

it('Does not have any a11y issues', async () => {
await page.addScriptTag({ path: require.resolve('axe-core') });
const results = await page.evaluate(
() =>
new Promise(resolve => {
window.axe.run((err, axeResults) => {
if (err) throw err;
resolve(axeResults);
});
})
);

const beautifyErrors = `
A11Y Violations:
${JSON.stringify(results.violations, null, 4)}`;
assert(results.violations.length === 0, beautifyErrors);
});
});
11 changes: 11 additions & 0 deletions packages/gestalt/src/Masonry/__integration__/.eslintrc.json
@@ -0,0 +1,11 @@
{
"globals": {
Copy link
Contributor

Choose a reason for hiding this comment

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

@christianvuerings i think this can be avoided, its baked into eslint now, you just need to add .integration.js to the overrides for jest env

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch - I'll do this in a follow up PR. I'd like to get this merged ASAP.

Copy link
Contributor

Choose a reason for hiding this comment

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

@chrislloyd @christianvuerings check out #193 which updates based on the instructions here

"afterAll": false,
"beforeAll": false,
"browser": false,
"describe": false,
"jest": false,
"it": false,
"page": false
}
}
@@ -1,51 +1,47 @@
/* global describe */
/* global it */
import assert from 'assert';
import ghost from 'ghostjs';
import selectors from './lib/selectors';
import triggerResize from './lib/triggerResize';

jest.setTimeout(10000);

const getItemColumnMap = async () => {
const gridItems = await ghost.findElements(selectors.gridItem);
const gridItems = await page.$$(selectors.gridItem);
const itemLeftMap = {};
for (let i = 0; i < gridItems.length; i += 1) {
const isVisible = await gridItems[i].isVisible();
if (isVisible) {
const itemRect = await gridItems[i].rect();
itemLeftMap[itemRect.left] = itemLeftMap[itemRect.left] || [];
itemLeftMap[itemRect.left].push({
...itemRect,
itemIndex: i,
text: await gridItems[i].text(),
});
}
const boundingBox = await gridItems[i].boundingBox();
itemLeftMap[boundingBox.x] = itemLeftMap[boundingBox.x] || [];
itemLeftMap[boundingBox.x].push({
...boundingBox,
itemIndex: i,
text: (await gridItems[i].getProperty('innerText')).toString(),
});
}

return itemLeftMap;
};

describe('Masonry > Flexible resize', () => {
it('Should resize item width and height on resize ', async () => {
ghost.close();
await ghost.open('http://localhost:3001/FlexibleMasonry', {
viewportSize: {
width: 800,
height: 800,
},
await page.setViewport({
width: 800,
height: 800,
});
await page.goto('http://localhost:3001/FlexibleMasonry');

// Wait for the grid to be hydrated.
// TODO: Break this out into a utility /w wait() instead.
await ghost.wait(1000);
await page.waitFor(1000);

// check size of initial grid items
const originalItemMap = await getItemColumnMap();
const originalColumns = Object.keys(originalItemMap);

// trigger slight resize. enough to resize, but not reflow columns
await triggerResize(820);
// Mock out the window width for the next resize calculation.
await triggerResize(820, page);

// Wait for the resize measurements to be finished
await ghost.wait(() => ghost.script(() => window.RESIZE_MEASUREMENT_DONE));
await page.waitForFunction(() => window.RESIZE_MEASUREMENT_DONE);

const newItemMap = await getItemColumnMap();
const newColumns = Object.keys(newItemMap);
Expand Down

This file was deleted.

@@ -0,0 +1,39 @@
import assert from 'assert';
import selectors from './lib/selectors';

describe('Masonry > handle item updates', () => {
it('should correctly update grid item heights', async () => {
await page.setViewport({
width: 800,
height: 800,
});
await page.goto('http://localhost:3001/Masonry?virtualize=1');

// get initial size of first element
const gridItems = await page.$$(selectors.gridItem);
const itemRectBefore = await gridItems[0].boundingBox();

// trigger item expand
const pushTrigger = await page.$(selectors.expandGridItems);
await pushTrigger.click();

const gridItemsAfter = await page.$$(selectors.gridItem);
const itemRectAfter = await gridItemsAfter[0].boundingBox();

assert.ok(
itemRectAfter.height > itemRectBefore.height,
'item height should have increased'
);

// trigger item collapse
await pushTrigger.click();

const gridItemsAfterAfter = await page.$$(selectors.gridItem);
const itemRectAfterAfter = await gridItemsAfterAfter[0].boundingBox();

assert.ok(
itemRectAfterAfter.height === itemRectBefore.height,
'item height should have reverted to original'
);
});
});

This file was deleted.

@@ -0,0 +1,29 @@
import assert from 'assert';
import selectors from './lib/selectors';

describe('Masonry > handle offset update', () => {
it('Should correctly account for relative position changes', async () => {
let gridItems;
let firstItemText;

// First load the page with javascript disabled to get the item position
await page.setViewport({
width: 800,
height: 800,
});
await page.goto('http://localhost:3001/Masonry?virtualize=1');

const pushTrigger = await page.$(selectors.pushGridDown);
await pushTrigger.click();

await page.evaluate(() => window.scrollTo(0, 500));
gridItems = await page.$$(selectors.gridItem);
firstItemText = (await gridItems[0].getProperty('innerText')).toString();
assert.ok(firstItemText.includes('foo 0'));

await page.evaluate(() => window.scrollTo(0, 1000));
gridItems = await page.$$(selectors.gridItem);
firstItemText = (await gridItems[0].getProperty('innerText')).toString();
assert.ok(firstItemText.includes('foo 0'));
});
});