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.

Sign up for GitHub

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 abstraction logic for simplifying writing visual tests #685

Merged
merged 15 commits into from
Jul 26, 2024
Merged

Add abstraction logic for simplifying writing visual tests #685

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
65a08d4
Introduce abstraction for component rendering functions and support f…
KshitijThareja Jul 11, 2024
d2ea3d1
Initial workflow config
KshitijThareja Jul 14, 2024
8ce5fe8
Fix the initial workflow for visual tests job
KshitijThareja Jul 16, 2024
2eaabdb
Add automatic comments job for visual testing workflow
KshitijThareja Jul 17, 2024
310f75b
Fix lint errors
KshitijThareja Jul 17, 2024
f6c66f3
Abstraction for page navigation
KshitijThareja Jul 17, 2024
1eba82f
Fix lint errors
KshitijThareja Jul 17, 2024
c37ddd3
Merge branch 'gsoc/visual-testing' into vt-abstraction
KshitijThareja Jul 17, 2024
f4373a0
Introduce visual testing to the existing Javascript tests workflow
KshitijThareja Jul 17, 2024
bdea16d
Introduce visual testing to the existing Javascript tests workflow
KshitijThareja Jul 17, 2024
c0e977f
Replace usage of custom describe blocks with test regex patterns
KshitijThareja Jul 20, 2024
95228a2
Merge branch 'vt-abstraction' of github.com:KshitijThareja/kolibri-de…
KshitijThareja Jul 20, 2024
61c98f9
Remove unintentional changes
KshitijThareja Jul 20, 2024
fde97db
Add comments for rendering function and make variable names clear
KshitijThareja Jul 23, 2024
93f2b7c
Add special test blocks for visual tests to append Visual tag automat…
KshitijThareja Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ esLintConfig.settings['import/resolver'].nuxt = {
nuxtSrcDir: 'docs',
};

// Remove linting errors for the globals defined in the jest-puppeteer package
// Remove linting errors for the globals defined in the jest-puppeteer package and testUtils
esLintConfig.globals = {
...esLintConfig.globals,
page: true,
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@ jobs:
yarn --frozen-lockfile
npm rebuild node-sass
- name: Run tests
run: yarn test
run: yarn test
11 changes: 0 additions & 11 deletions jest.conf/testUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,6 @@ export const resizeWindow = (width, height = 768) => {
});
};

export function canTakeScreenshot() {
const percyToken = process.env.PERCY_TOKEN;
const runVisualTests = process.env.TEST_TYPE === 'visual';
if (runVisualTests && !percyToken) {
throw new Error(
'Error: Visual tests cannot be run because PERCY_TOKEN environment variable is not set.'
);
}
return runVisualTests && percyToken;
}

export const testAfterResize = testFunction => {
let animationFrameId;
const assertAfterResize = () => {
Expand Down
5 changes: 5 additions & 0 deletions jest.conf/visual.setup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,9 @@ import { percySnapshot } from '@percy/puppeteer';
// Set the test type to visual
process.env.TEST_TYPE = 'visual';

const TESTING_PLAYGROUND_URL = 'http://localhost:4000/testing-playground';
global.percySnapshot = percySnapshot;

global.beforeAll(async () => {
await page.goto(TESTING_PLAYGROUND_URL, { waitUntil: 'networkidle2' });
AlexVelezLl marked this conversation as resolved.
Show resolved Hide resolved
});
43 changes: 43 additions & 0 deletions jest.conf/visual.testUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import percySnapshot from '@percy/puppeteer';

export async function renderComponent(component, props) {
const initialState = await page.evaluate(() => {
AlexVelezLl marked this conversation as resolved.
Show resolved Hide resolved
const testing_playground = document.querySelector('#testing-playground');
return testing_playground ? testing_playground.innerHTML : '';
});
await page.evaluate(
({ component, props }) => {
window.postMessage(
{
type: 'RENDER_COMPONENT',
component: component,
props: props,
},
'*'
);
},
{ component, props }
);
await page.waitForSelector('#testing-playground');

await page.waitForFunction(
AlexVelezLl marked this conversation as resolved.
Show resolved Hide resolved
initialState => {
const testing_playground = document.querySelector('#testing-playground');
return testing_playground && testing_playground.innerHTML !== initialState;
},
{},
initialState
);
const isComponentRendered = await page.evaluate(initialState => {
const testing_playground = document.querySelector('#testing-playground');
return testing_playground && testing_playground.innerHTML !== initialState;
}, initialState);

global.expect(isComponentRendered).toBe(true);
}

export async function takeSnapshot(name) {
if (process.env.TEST_TYPE == 'visual') {
await percySnapshot(page, name);
}
}
146 changes: 56 additions & 90 deletions lib/buttons-and-links/__tests__/KButton.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,109 +1,75 @@
import { shallowMount } from '@vue/test-utils';
import percySnapshot from '@percy/puppeteer';
import KButton from '../KButton.vue';
import { canTakeScreenshot } from '../../../jest.conf/testUtils';
import { renderComponent, takeSnapshot } from '../../../jest.conf/visual.testUtils';

describe('KButton', () => {
if (!canTakeScreenshot()) {
describe('icon related props', () => {
it('should render an icon before the text with the icon string passed to the `icon` prop', () => {
const wrapper = shallowMount(KButton, {
propsData: {
icon: 'add',
},
});
expect(wrapper.find('[data-test="iconBefore"]').exists()).toBe(true);
describe('icon related props', () => {
it('should render an icon before the text with the icon string passed to the `icon` prop', () => {
const wrapper = shallowMount(KButton, {
propsData: {
icon: 'add',
},
});
it('should render an icon after the text with the icon string pased to the `iconAfter` prop', () => {
const wrapper = shallowMount(KButton, {
propsData: {
iconAfter: 'video',
},
});
expect(wrapper.find('[data-test="iconAfter"]').exists()).toBe(true);
expect(wrapper.find('[data-test="iconBefore"]').exists()).toBe(true);
});
it('should render an icon after the text with the icon string pased to the `iconAfter` prop', () => {
const wrapper = shallowMount(KButton, {
propsData: {
iconAfter: 'video',
},
});
it('should render a dropdown icon when hasDropdown is true', () => {
const wrapper = shallowMount(KButton, {
propsData: {
hasDropdown: true,
},
});
expect(wrapper.find('[data-test="dropdownIcon"]').exists()).toBe(true);
expect(wrapper.find('[data-test="iconAfter"]').exists()).toBe(true);
});
it('should render a dropdown icon when hasDropdown is true', () => {
const wrapper = shallowMount(KButton, {
propsData: {
hasDropdown: true,
},
});
expect(wrapper.find('[data-test="dropdownIcon"]').exists()).toBe(true);
});
});

describe('text prop and slots', () => {
it('should render the text prop if nothing is in the default slot', () => {
const wrapper = shallowMount(KButton, {
propsData: {
text: 'test',
},
});
expect(wrapper.text()).toContain('test');
describe('text prop and slots', () => {
it('should render the text prop if nothing is in the default slot', () => {
const wrapper = shallowMount(KButton, {
propsData: {
text: 'test',
},
});
expect(wrapper.text()).toContain('test');
});

it('should render the slot when the slot has content', () => {
const wrapper = shallowMount(KButton, {
propsData: {
text: 'test',
},
slots: {
default: '<span>slot</span>',
},
});
expect(wrapper.text()).toContain('slot');
expect(wrapper.text()).toContain('test');
it('should render the slot when the slot has content', () => {
const wrapper = shallowMount(KButton, {
propsData: {
text: 'test',
},
slots: {
default: '<span>slot</span>',
},
});
expect(wrapper.text()).toContain('slot');
expect(wrapper.text()).toContain('test');
});
});

describe('event handling', () => {
it('should emit a click event when clicked', () => {
const wrapper = shallowMount(KButton, {
propsData: {
text: 'test',
},
});
wrapper.trigger('click');
expect(wrapper.emitted().click).toBeTruthy();
describe('event handling', () => {
it('should emit a click event when clicked', () => {
const wrapper = shallowMount(KButton, {
propsData: {
text: 'test',
},
});
wrapper.trigger('click');
expect(wrapper.emitted().click).toBeTruthy();
});
} else {
describe('KButton Visual Tests', () => {
beforeAll(async () => {
await page.goto('http://localhost:4000/testing-playground', { waitUntil: 'networkidle2' });
});
});

async function renderComponent(component, props) {
await page.evaluate(
({ component, props }) => {
window.postMessage(
{
type: 'RENDER_COMPONENT',
component: component,
props: props,
},
'*'
);
},
{ component, props }
);
await page.waitForSelector('#testing-playground');

const isComponentRendered = await page.evaluate(() => {
const testing_playground = document.querySelector('#testing-playground');
return testing_playground && testing_playground.children.length > 0;
});

if (!isComponentRendered) {
// eslint-disable-next-line no-console
console.error('Component did not render in the testing playground');
}
}

it('renders correctly with default props', async () => {
await renderComponent('KButton', { text: 'Test Button' });
await percySnapshot(page, 'KButton - Default');
});
describe('[Visual] KButton Visual Tests', () => {
it('renders correctly with default props', async () => {
AlexVelezLl marked this conversation as resolved.
Show resolved Hide resolved
await renderComponent('KButton', { text: 'Test Button' });
await takeSnapshot('KButton - Default');
});
}
});
});
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
"precompile-svgs": "node utils/precompileSvgs/index.js && yarn run pregenerate",
"precompile-custom-svgs": "node utils/precompileSvgs/index.js --custom && yarn run pregenerate",
"_lint-watch-fix": "yarn lint -w -m",
"test:percy": "PERCY_LOGLEVEL=error npx percy exec -v -- jest --config jest.conf/visual.index.js -i ./lib/buttons-and-links/__tests__/KButton.spec.js",
"test": "jest --config=jest.conf/index.js",
"test:percy": "PERCY_LOGLEVEL=info npx percy exec -v -- jest --config jest.conf/visual.index.js -i ./lib/buttons-and-links/__tests__/KButton.spec.js -t '\\[Visual\\]'",
"test": "jest --config=jest.conf/index.js -t '^(?!.*\\[Visual\\])'",
Copy link
Member

Choose a reason for hiding this comment

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

To avoid complex regex patterns, we can also use the --testPathIgnorePatterns option, so we just replicate the regex of the previous script just changing the option flag -t with --testPathIgnorePatterns https://jestjs.io/docs/cli#--testpathignorepatternsregexarray

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Will try it out :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just read that the flag --testPathIgnorePatterns is used to ignore specific test files based on their path and not the tests within it. Maybe if we had something like --testNameIgnorePatterns it would have been easier.

Copy link
Member

Choose a reason for hiding this comment

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

Ooooh it's true, I didn't realize, it seems that jest doesn't have an ignore name pattern. Well, we could keep the regex then

Copy link
Member

Choose a reason for hiding this comment

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

Just tested the regex and it seems to be ok 🎉

"test:visual": "concurrently --kill-others --success first --names \"SERVER,TEST\" -c \"bgCyan.bold,bgYellow.bold\" \"yarn dev-only > /dev/null 2>&1\" \"yarn test:percy\"",
"_api-watch": "chokidar \"**/lib/**\" -c \"node utils/extractApi.js\""
},
Expand Down
55 changes: 2 additions & 53 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5778,7 +5778,7 @@ dotenv@^9.0.2:
resolved "https://registry.yarnpkg.com/dotenv/-/dotenv-9.0.2.tgz#dacc20160935a37dea6364aa1bef819fb9b6ab05"
integrity sha512-I9OvvrHp4pIARv4+x9iuewrWycX6CcZtoAu1XrzPxc5UygMJXJZYmBsynku8IkrJwgypE5DGNjDPmPRhDCptUg==

duplexer@^0.1.2, duplexer@~0.1.1:
duplexer@^0.1.2:
version "0.1.2"
resolved "https://registry.yarnpkg.com/duplexer/-/duplexer-0.1.2.tgz#3abe43aef3835f8ae077d136ddce0f276b0400e6"
integrity sha512-jtD6YG370ZCIi/9GTaJKQxWTZD045+4R4hTk/x1UyoqadyJ9x9CgSi1RlVDQF8U2sxLLSnFkCaMihqljHIWgMg==
Expand Down Expand Up @@ -6338,19 +6338,6 @@ etag@^1.8.1, etag@~1.8.1:
resolved "https://registry.npmjs.org/etag/-/etag-1.8.1.tgz"
integrity sha512-aIL5Fx7mawVa300al2BnEE4iNvo1qETxLrPI/o05L7z6go7fCw1J6EQmbK4FmJ2AS7kgVF/KEZWufBfdClMcPg==

event-stream@=3.3.4:
version "3.3.4"
resolved "https://registry.yarnpkg.com/event-stream/-/event-stream-3.3.4.tgz#4ab4c9a0f5a54db9338b4c34d86bfce8f4b35571"
integrity sha512-QHpkERcGsR0T7Qm3HNJSyXKEEj8AHNxkY3PK8TS2KJvQ7NiSHe3DDpwVKKtoYprL/AreyzFBeIkBIWChAqn60g==
dependencies:
duplexer "~0.1.1"
from "~0"
map-stream "~0.1.0"
pause-stream "0.0.11"
split "0.3"
stream-combiner "~0.0.4"
through "~2.3.1"

eventemitter3@^4.0.0:
version "4.0.7"
resolved "https://registry.npmjs.org/eventemitter3/-/eventemitter3-4.0.7.tgz"
Expand Down Expand Up @@ -6890,11 +6877,6 @@ from2@^2.1.0:
inherits "^2.0.1"
readable-stream "^2.0.0"

from@~0:
version "0.1.7"
resolved "https://registry.yarnpkg.com/from/-/from-0.1.7.tgz#83c60afc58b9c56997007ed1a768b3ab303a44fe"
integrity sha512-twe20eF1OxVxp/ML/kq2p1uc6KvFK/+vs8WjEbeKmV2He22MKm7YF2ANIt+EOqhJ5L3K/SuuPhk0hWQDjOM23g==

fs-exists-sync@^0.1.0:
version "0.1.0"
resolved "https://registry.yarnpkg.com/fs-exists-sync/-/fs-exists-sync-0.1.0.tgz#982d6893af918e72d08dec9e8673ff2b5a8d6add"
Expand Down Expand Up @@ -9604,11 +9586,6 @@ map-obj@^4.0.0:
resolved "https://registry.yarnpkg.com/map-obj/-/map-obj-4.3.0.tgz#9304f906e93faae70880da102a9f1df0ea8bb05a"
integrity sha512-hdN1wVrZbb29eBGiGjJbeP8JbKjq1urkHJ/LIP/NY48MZ1QVXUsQBV1G1zvYFHn1XE06cwjBsOI2K3Ulnj1YXQ==

map-stream@~0.1.0:
version "0.1.0"
resolved "https://registry.yarnpkg.com/map-stream/-/map-stream-0.1.0.tgz#e56aa94c4c8055a16404a0674b78f215f7c8e194"
integrity sha512-CkYQrPYZfWnu/DAmVCpTSX/xHpKZ80eKh2lAkyA6AJTef6bW+6JpbQZN5rofum7da+SyN1bi5ctTm+lTfcCW3g==

map-values@^1.0.1:
version "1.0.1"
resolved "https://registry.npmjs.org/map-values/-/map-values-1.0.1.tgz"
Expand Down Expand Up @@ -10881,13 +10858,6 @@ path-type@^4.0.0:
resolved "https://registry.npmjs.org/path-type/-/path-type-4.0.0.tgz"
integrity sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw==

[email protected]:
version "0.0.11"
resolved "https://registry.yarnpkg.com/pause-stream/-/pause-stream-0.0.11.tgz#fe5a34b0cbce12b5aa6a2b403ee2e73b602f1445"
integrity sha512-e3FBlXLmN/D1S+zHzanP4E/4Z60oFAa3O051qt1pxa7DEJWKAyil6upYVXCWadEnuoqa4Pkc9oUx9zsxYeRv8A==
dependencies:
through "~2.3"

pbkdf2@^3.0.3, pbkdf2@^3.1.2:
version "3.1.2"
resolved "https://registry.npmjs.org/pbkdf2/-/pbkdf2-3.1.2.tgz"
Expand Down Expand Up @@ -12083,13 +12053,6 @@ prr@~1.0.1:
resolved "https://registry.npmjs.org/prr/-/prr-1.0.1.tgz"
integrity sha1-0/wRS6BplaRexok/SEzrHXj19HY=

ps-tree@^1.2.0:
version "1.2.0"
resolved "https://registry.yarnpkg.com/ps-tree/-/ps-tree-1.2.0.tgz#5e7425b89508736cdd4f2224d028f7bb3f722ebd"
integrity sha512-0VnamPPYHl4uaU/nSFeZZpR21QAWRz+sRv4iW9+v/GS/J5U5iZB5BNN6J0RMoOvdx2gWM2+ZFMIm58q24e4UYA==
dependencies:
event-stream "=3.3.4"

pseudomap@^1.0.2:
version "1.0.2"
resolved "https://registry.npmjs.org/pseudomap/-/pseudomap-1.0.2.tgz"
Expand Down Expand Up @@ -13508,13 +13471,6 @@ split-string@^3.0.1, split-string@^3.0.2:
dependencies:
extend-shallow "^3.0.0"

[email protected]:
version "0.3.3"
resolved "https://registry.yarnpkg.com/split/-/split-0.3.3.tgz#cd0eea5e63a211dfff7eb0f091c4133e2d0dd28f"
integrity sha512-wD2AeVmxXRBoX44wAycgjVpMhvbwdI2aZjCkvfNcH1YqHQvJVa1duWc73OyVGJUc05fhFaTZeQ/PYsrmyH0JVA==
dependencies:
through "2"

sprintf-js@^1.1.3:
version "1.1.3"
resolved "https://registry.yarnpkg.com/sprintf-js/-/sprintf-js-1.1.3.tgz#4914b903a2f8b685d17fdf78a70e917e872e444a"
Expand Down Expand Up @@ -13620,13 +13576,6 @@ stream-browserify@^2.0.1:
inherits "~2.0.1"
readable-stream "^2.0.2"

stream-combiner@~0.0.4:
version "0.0.4"
resolved "https://registry.yarnpkg.com/stream-combiner/-/stream-combiner-0.0.4.tgz#4d5e433c185261dde623ca3f44c586bcf5c4ad14"
integrity sha512-rT00SPnTVyRsaSz5zgSPma/aHSOic5U1prhYdRy5HS2kTZviFpmDgzilbtsJsxiroqACmayynDN/9VzIbX5DOw==
dependencies:
duplexer "~0.1.1"

stream-each@^1.1.0:
version "1.2.3"
resolved "https://registry.npmjs.org/stream-each/-/stream-each-1.2.3.tgz"
Expand Down Expand Up @@ -14254,7 +14203,7 @@ through2@^2.0.0:
readable-stream "~2.3.6"
xtend "~4.0.1"

through@2, through@^2.3.6, through@^2.3.8, through@~2.3, through@~2.3.1:
through@^2.3.6, through@^2.3.8:
version "2.3.8"
resolved "https://registry.npmjs.org/through/-/through-2.3.8.tgz"
integrity sha1-DdTJ/6q8NXlgsbckEV1+Doai4fU=
Expand Down
Loading