Integrate visual testing mechanism to the latest KDS version #901

KshitijThareja · 2025-01-20T21:11:19Z

Description

This PR aims to integrate the visual testing mechanism to the latest version of KDS and ensure its proper working by implementing necessary changes.

Changelog

Description: Integrates visual testing setup to KDS.
Products impact: -
Addresses: -
Components: -
Breaking: -
Impacts a11y: -
Guidance: -

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical and brittle code paths are covered by unit tests
The change is described in the changelog section above

Reviewer guidance

Is the code clean and well-commented?
Are there tests for this change?
Are all UI components LTR and RTL compliant (if applicable)?
Add other things to check for here

…sting Percy and jest-puppeteer environment setup for visual testing

…or custom describe blocks

…ncurrently Replace custom checks from the server script to use concurrently

…ically

…ction Add abstraction logic for simplifying writing visual tests

…le template

Implement visual tests for KButton component

bjester

A few comments, but nothing blocking that I can see from a quick pass

bjester · 2025-01-21T15:06:24Z

.github/workflows/visual_tests.yml

+          yarn --frozen-lockfile
+          npm rebuild node-sass
+      - name: Download Chromium
+        run: npx puppeteer browsers install chrome


Curious if this is cached, and if not, if we should

From what I can infer from the test run, puppeteer does cache the browser after the first workflow run and uses the same for successive runs. Is there a need to add another step to the workflow for the same?

bjester · 2025-01-21T15:07:40Z

jest.conf/visual.index.js

@@ -0,0 +1,121 @@
+const path = require('node:path');
+const http = require('http');


This should probably be node:http

bjester · 2025-01-21T15:08:51Z

lib/buttons-and-links/__tests__/KButton.spec.js

@@ -64,4 +65,106 @@ describe('KButton', () => {
      expect(wrapper.emitted().click).toBeTruthy();
    });
  });
+
+  describe.visual('KButton Visual Tests', () => {


I suppose Visual in the string is redundant with what describe.visual does

I'll just make it KButton Tests in that case 👍

AlexVelezLl · 2025-01-27T15:29:14Z

lib/KThemePlugin.js

@@ -60,7 +60,7 @@ require('./grids/globalStyles.js'); // global grid styles
 * Also, set up global state, listeners, and styles.
 */
 export default function KThemePlugin(Vue) {
-  if (!isNuxtServerSideRendering()) {
+  if (!isNuxtServerSideRendering() && process.env.VISUAL_TESTING !== 'true') {


Why wouldnt we install the KThemePlugin while running visual tests?

With the introduction of KLiveRegion and the changes in the KThemePlugin, if we try to run visual tests, the variable 'document' will become undefined as for it to work as intended, we require a jsdom test environment. This will result in errors in visual test execution

Alright, thanks! Lets merge it, let it run and then if we need to activate it, we can look for alternatives. Thanks!

AlexVelezLl · 2025-01-29T18:59:07Z

Thanks a lot for all your hard work @KshitijThareja!!! Merging! 🎉 🎉 🎉.

AlexVelezLl · 2025-01-29T19:25:08Z

Hey @KshitijThareja @bjester, we are getting this error on the execution. Do you have any idea why could it be happening? https://github.com/learningequality/kolibri-design-system/actions/runs/13038589590/job/36375439798

bjester · 2025-01-29T19:49:10Z

@AlexVelezLl ChatGPT says to possibly try using --no-sandbox first, and then possibly add --disable-setuid-sandbox. Also, we should ensure all dependencies are installed in the action's container

KshitijThareja · 2025-01-29T20:31:15Z

Hi @AlexVelezLl @bjester
So I looked it up and found this. The easiest, and probably the best fix for now would be to just change the ubuntu version used for running visual test workflow to 22.04.
Should I go forward with making a PR for the same?

bjester · 2025-01-29T21:41:26Z

Hi @AlexVelezLl @bjester So I looked it up and found this. The easiest, and probably the best fix for now would be to just change the ubuntu version used for running visual test workflow to 22.04. Should I go forward with making a PR for the same?

Sounds like --no-sandbox is easier:

What if I don‘t have root access to the machine and can’t install anything?
You will need to run developer builds with the --no-sandbox command line flag, but be aware that this disables critical security features of Chromium and should never be used when browsing the open web.

KshitijThareja and others added 30 commits January 19, 2025 01:02

Add and configure percy for visual tests

0236bf6

Change server configuration and file names for visual tests

2434700

Add component rendering mechanism for visual testing

b3f0bb4

Remove unused packages

7d55f4b

Add JSDoc for testing playground and modify server script

c91086f

Update KImg unit tests and eslint configuration

e251350

Use concurrently to manage startup and termination of tests

cefdfe6

Update waitForServer function to use http

63d5144

Remove unused dependencies and separate visual tests from yarn test

b6044ad

Separate visual tests from yarn test command

fc09854

Merge pull request learningequality#670 from KshitijThareja/visual-te…

cf41d8c

…sting Percy and jest-puppeteer environment setup for visual testing

Introduce abstraction for component rendering functions and support f…

6c3fd69

…or custom describe blocks

Initial workflow config

985c0d4

Fix the initial workflow for visual tests job

e5d792d

Merge pull request learningequality#677 from KshitijThareja/vt-add-co…

8823a8f

…ncurrently Replace custom checks from the server script to use concurrently

Add automatic comments job for visual testing workflow

cc9417c

Fix lint errors

2ef82b1

Abstraction for page navigation

9ad6f8f

Fix lint errors

1b024f9

Introduce visual testing to the existing Javascript tests workflow

4454b60

Replace usage of custom describe blocks with test regex patterns

8119ec9

Remove unintentional changes

e1a54ba

Add comments for rendering function and make variable names clear

f55a831

Remove run server step

475827f

Remove pipes where not required

e325e34

Add special test blocks for visual tests to append Visual tag automat…

622a22c

…ically

Merge pull request learningequality#685 from KshitijThareja/vt-abstra…

55bff15

…ction Add abstraction logic for simplifying writing visual tests

Add support for default slots rendering

02d0dbb

Change workflow to run on separate environment

7c117e7

Merge branch 'gsoc/visual-testing' into vt-ghaction

2d2a9bd

AlexVelezLl and others added 15 commits January 19, 2025 01:26

Merge branch 'gsoc/visual-testing' into vt-test

1c7ec1e

Modify testing playground to render named and default slots in a sing…

7d6c634

…le template

Add jsdocs for renderComponent and takeSnapshot functions

09820a1

Fix lint error

05063ac

Remove async from describe block

aaa4411

Add snapshot options specifying test widths

6be488b

Merge pull request learningequality#710 from KshitijThareja/vt-test

15d8ae5

Implement visual tests for KButton component

Update test command to work for all test files

1f09d07

Add additional info for rendering complex components

083532a

Add link to percy documentation

86d7402

Update info for usage of test blocks for visual tests

e4b8986

Update required node version specification

c8e4d0e

Fix errors in integrating visual testing setup with the codebase

b0cc1e7

Update percy and puppeteer to latest versions

924f782

Update package.json

d07775a

MisRob requested review from bjester and AlexVelezLl January 21, 2025 14:20

bjester approved these changes Jan 21, 2025

View reviewed changes

KshitijThareja and others added 3 commits January 24, 2025 02:40

Update visual test description for KButton tests and fix http import

7b7395d

Resolve merge conflicts with the develop branch

78324ba

Merge branch 'develop' into gsoc/vis-testing

d4d12b1

AlexVelezLl reviewed Jan 27, 2025

View reviewed changes

nucleogenesis assigned AlexVelezLl Jan 28, 2025

AlexVelezLl merged commit f82e7e0 into learningequality:develop Jan 29, 2025
9 checks passed

learning-equality-bot bot pushed a commit that referenced this pull request Jan 29, 2025

Update CHANGELOG.md for PR #901

61b2a12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate visual testing mechanism to the latest KDS version #901

Integrate visual testing mechanism to the latest KDS version #901

KshitijThareja commented Jan 20, 2025

bjester left a comment

bjester Jan 21, 2025

KshitijThareja Jan 21, 2025

bjester Jan 21, 2025

bjester Jan 21, 2025

KshitijThareja Jan 21, 2025

AlexVelezLl Jan 27, 2025

KshitijThareja Jan 29, 2025

AlexVelezLl Jan 29, 2025

AlexVelezLl commented Jan 29, 2025

AlexVelezLl commented Jan 29, 2025

bjester commented Jan 29, 2025

KshitijThareja commented Jan 29, 2025

bjester commented Jan 29, 2025

		@@ -0,0 +1,121 @@
		const path = require('node:path');
		const http = require('http');

Integrate visual testing mechanism to the latest KDS version #901

Integrate visual testing mechanism to the latest KDS version #901

Conversation

KshitijThareja commented Jan 20, 2025

Description

Changelog

Testing checklist

Reviewer guidance

bjester left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexVelezLl commented Jan 29, 2025

AlexVelezLl commented Jan 29, 2025

bjester commented Jan 29, 2025

KshitijThareja commented Jan 29, 2025

bjester commented Jan 29, 2025