-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
add integration test in CI #151
Conversation
Main issue with this one is the font rendering which makes the tests fail due to too many differences between my local image files and the server generated ones. I believe the best way to achieve this is to setup our local to use docker and run the test inside the same container image as the server. Good place to start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can kill the __image_snapshots__
folder, that will be awesome!
I have added the ARGOS_TOKEN
env variable in the Circle CI configuration.
@@ -11,7 +11,7 @@ defaults: &defaults | |||
REACT_DIST_TAG: << parameters.react-dist-tag >> | |||
working_directory: /tmp/material-ui-x | |||
docker: | |||
- image: circleci/node:10 | |||
- image: circleci/node:latest-browsers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefer to run the tests with v10, as the closest version to the minimum version of node we support for the components. Right now, we support node v8, but plan to upgrade mui/material-ui#19301 (comment).
Also, in theory, we don't need to load this distribution, puppeteer comes with a binary of chrome in node_modules/puppeteer/.local-chromium
- image: circleci/node:latest-browsers | |
- image: circleci/node:10 |
For the missing dependencies, we have prepare_chrome_headless
the step which should work with the non headless version too
@@ -81,6 +81,7 @@ | |||
"test:karma": "cross-env NODE_ENV=test karma start test/karma.conf.js", | |||
"test:unit": "cross-env NODE_ENV=test mocha 'packages/**/*.test.tsx' --exclude '**/node_modules/**'", | |||
"test:watch": "yarn test:unit --watch", | |||
"test:e2e": "cd packages/storybook && yarn test:ci", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering storybook is a workspace?
"test:e2e": "cd packages/storybook && yarn test:ci", | |
"test:e2e": "yarn workspace storybook test:ci", |
@@ -31,4 +31,7 @@ describe('Components override', () => { | |||
await snapshotTest('/story/x-grid-demos-custom-components--styled-columns'); | |||
done(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done(); |
@@ -31,4 +31,7 @@ describe('Components override', () => { | |||
await snapshotTest('/story/x-grid-demos-custom-components--styled-columns'); | |||
done(); | |||
}); | |||
afterAll(async () => { | |||
await stopBrowser(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused, why are startBrowser
and stopBrowser
needed? Isn't this feature provided by https://github.com/smooth-code/jest-puppeteer/blob/69a0c3ee80ef4b6af08d45ff44229df6ced8b666/packages/jest-puppeteer-preset/jest-preset.json#L2 in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No as it's puppeter the web driver so you need to load an instance of the browser from its package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get it. From what I understand, we use jest-puppeteer, which is meant to handle the launch and close of the browser:
- The preset has a setup command https://github.com/smooth-code/jest-puppeteer/blob/56715a8938e859d994c9ea1520cf44a3c796c98e/packages/jest-puppeteer-preset/jest-preset.json#L2
- The setup command launch and close the brower: https://github.com/smooth-code/jest-puppeteer/blob/56715a8938e859d994c9ea1520cf44a3c796c98e/packages/jest-environment-puppeteer/src/global.js#L21
@@ -20,15 +24,33 @@ export const IMAGE_SNAPSHOT_CONFIG = { | |||
}, | |||
} as MatchImageSnapshotOptions; | |||
|
|||
let browserInstance; | |||
export async function startBrowser() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look at the helpers I wrote in the past to write tests with puppeteer, if that help:
Usage, createContext()
import config from 'config'
import { ACCOUNT_STATUSES, SCOPES } from 'api/constants'
import { createContext } from 'test/e2e/setup'
import signIn from 'test/e2e/modules/signIn'
import matchers from 'test/e2e/modules/matchers'
import screenshot from 'test/e2e/modules/screenshot'
describe('signIn', () => {
const context = createContext()
it('should work', async () => {
const account = await context.factory.create(
'Account',
{ status: ACCOUNT_STATUSES.APPROVED },
{ scopes: [SCOPES.CONTRIBUTOR] }
)
const { page, i18n } = context
await page.goto(`${config.get('contributor.url')}/sign-in`)
await matchers.toMatch(page, i18n.t('contributorSignIn:apply'), {
timeout: 'navigation',
})
await screenshot(page, 'contributor-sign-in')
await page.type('input[name="email"]', account.email)
await page.type('input[name="password"]', account.password)
await page.type('input[name="password"]', '\u000d')
await matchers.toMatchUrl(page, `${config.get('contributor.url')}/`)
await matchers.toMatch(page, i18n.t('contributorDashboard:imageRequiresEditing'), {
timeout: 'navigation',
})
await page.waitFor('[data-e2e="appBar.user.menu"]')
await screenshot(page, 'contributor-dashboard')
await page.click('[data-e2e="appBar.user.menu"]')
await matchers.toMatchElement(page, '[role="menu"] > div:first-child', {
text: `${account.firstName} ${account.lastName}`,
})
})
it('should redirect when trying to access the sign in page', async () => {
const account = await context.factory.create(
'Account',
{ status: ACCOUNT_STATUSES.APPROVED },
{ scopes: [SCOPES.CONTRIBUTOR] }
)
await signIn({ context, account, scope: SCOPES.CONTRIBUTOR })
const { page } = context
await page.goto(`${config.get('contributor.url')}/settings/profile`)
expect(page.url()).toBe(`${config.get('contributor.url')}/settings/profile`)
await page.goto(`${config.get('contributor.url')}/sign-in`)
expect(page.url()).toBe(`${config.get('contributor.url')}/`)
})
})
test/e2e/setup.js
/* eslint-disable no-underscore-dangle, import/prefer-default-export, import/no-extraneous-dependencies */
/* global jasmine */
import fetch from 'isomorphic-fetch'
import knexfile from 'knexfile'
import puppeteer from 'puppeteer'
import log from 'modules/scripts/log'
import waitUntil from 'modules/async/waitUntil'
import hackTranslate from 'modules/i18n/hackTranslate'
import i18nInit from 'web/modules/i18n/server'
import getFactory from 'test/modules/api/factory'
import createKnex from 'api/services/database/createKnex'
import truncateAll from 'api/services/database/truncateAll'
import { IMAGE_STATUSES } from 'api/constants'
jasmine.DEFAULT_TIMEOUT_INTERVAL = 6000e3
async function getBrowserWSEndpoint() {
if (process.env.__BROWSER_WS_ENDPOINT__) {
return process.env.__BROWSER_WS_ENDPOINT__
}
const browserRemoteUrl = process.env.BROWSER_REMOTE_URL
if (!browserRemoteUrl) {
throw new Error('process.env.BROWSER_REMOTE_URL is missing')
}
const browserWSEndpoint = await waitUntil(async () => {
log.info({
name: 'browser',
msg: `Fetch data from ${browserRemoteUrl}`,
force: true,
})
let version
try {
version = await fetch(`${browserRemoteUrl}/json/version`, {
method: 'GET',
}).then(result => result.json())
} catch (err) {
// ...
}
return {
predicate: version,
result: version ? version.webSocketDebuggerUrl : null,
}
})
log.info({
name: 'browser',
msg: `Resolved browser endpoint ${browserWSEndpoint} `,
force: true,
})
process.env.__BROWSER_WS_ENDPOINT__ = browserWSEndpoint
return browserWSEndpoint
}
async function launchBrowser() {
const browserWSEndpoint = await getBrowserWSEndpoint()
return puppeteer.connect({ browserWSEndpoint, slowMo: 0 })
}
function handleError(err) {
throw err
}
const knexConfig = knexfile[process.env.NODE_ENV]
export function createContext() {
const context = {}
const screenSize = process.env.SCREEN_SIZE
if (!screenSize) {
throw new Error('Missing process.env.SCREEN_SIZE')
}
const [width, height] = screenSize.split('x').map(Number)
beforeAll(async () => {
// console.time('db')
context.knex = createKnex({ connection: { database: knexConfig.connection.database } })
context.factory = getFactory(context.knex.models)
// console.timeEnd('db')
await Promise.all([
truncateAll(context.knex),
(async () => {
// console.time('browser')
context.browser = await launchBrowser()
const pages = await context.browser.pages()
if (!pages.length) {
pages.push(await context.browser.newPage())
}
context.page = pages[0]
context.page.setDefaultNavigationTimeout(
process.env.NODE_ENV === 'production' ? 10e3 : 60e3
)
context.page.addListener('pageerror', handleError)
await context.page.setViewport({ width, height: height - 83 })
// console.timeEnd('browser')
})(),
(async () => {
// console.time('i18n')
const i18n = await i18nInit()
context.i18n = hackTranslate(i18n)
// console.timeEnd('i18n')
})(),
])
})
// Avoid duplicating beforeAll truncate
let truncateDo = false
beforeEach(async () => {
// Kill all the in-flight requests
const html =
'<!DOCTYPE html><html><head></head><body>beforeEach, kill all the pending requests</body></html>'
await context.page.goto(`data:text/html,${html}`)
await Promise.all([
(async () => {
if (truncateDo) {
await truncateAll(context.knex)
}
truncateDo = true
})(),
context.page._client.send('Network.clearBrowserCookies'),
])
await context.factory.create('Image', {
customerHomeBackground: true,
status: IMAGE_STATUSES.APPROVED_ONLINE,
})
})
afterAll(async () => {
context.page.removeListener('pageerror', handleError)
await Promise.all([context.browser.disconnect(), context.knex.destroy()])
})
return context
}
Regarding 22d8b0c, here is my experience with doing visual regression tests with the current setup (running with a Chrome installed on CircleCI). We had stable visual diff between two runs of the continuous integration, it was enough to only leverage the isolated environment CircleCI provides. My previous paragraph advocating against using Docker assumes that running the visual regression tests locally is interesting in less than 1% of the pull requests, my experience so far. A workflow that we have battle-tested and has been working great is:
|
Closing this one as we have not worked on it since August and we need to reevaluate our approach on how to integrate the tests in the CI. |
Part of #37