Vocabulary: Write unit and visual regression tests

[dhruvkb]

Description
The project currently does not define tests for any of the components. That, although okay in the present, would pose problems down the line when a lot of contributors would be working on the project. So the goal is to add unit testing to the repository and also setup a CircleCI task to run these tests on every PR.

Problems solved
Tests make it easy to identify breaking changes and fix them as and when they occur.

Alternatives
Alternatives include Storybook Snapshots. As a matter of fact, Snapshots should be added _in addition to_ these tests and not as a replacement for.

[Tanuj22]

@dhruvkb Can I work on this?

[dhruvkb]

Before you start, it would be a good idea to explain your approach and the tech you'll be using.

[Tanuj22]

@dhruvkb I am new to testing but this is what I found for Storybook Snapshots(https://www.npmjs.com/package/@storybook/addon-storyshots). In unit testing do we need to check the CSS? If yes, this is what I found https://storybook.js.org/docs/testing/react-ui-testing/#3-cssstyle-testing . Am I heading in the right direction?

[dhruvkb]

@Tanuj22 storyshots looks like a good place to start. Make a PR to set up storyshots with just one component as a pilot.

I also made this PR multiple-contributor so that we can have other contributors setting up other testing tools and writing tests.

[silvinabt87]

I'd like to contribute to this issue too, if possible. I'm currently investigating about it using the links @dhruvkb and other contributors provided. Is there any advice you would give me to work on this? (apart from the info given on this thread and on the chat)

[jahnvigupta]

Do you want test to make sure that the designs are according to your requirement? If yes, then can I get the link to your requirements?

[dhruvkb]

I've asked @Tanuj22 to setup the Storyshots addon for our Storybook. That addon and the snapshots that it takes will be our visual tests.

@silvinabt87 you can look into True which is a unit testing library for Sass. You can setup the framework and write some pilot unit tests to see whether the token classes in color.scss, spacing.scss and typography.scss are generated correctly.

@jahnvigupta our designs are completely based on the Figma Design Library. That is our one and only requirement and we'll be testing our components against that.

[kushaggarwal]

Hello @dhruvkb
Just to get a bit of a clear picture for myself can you please help as if what are all the things that we are aiming to develop test for under unit testing for this
Thank you

[dhruvkb]

We use Sass as our CSS pre-processor. We don't write raw CSS. So with unit testing we are trying to make sure that post conversion, the CSS meets some rules that we will define.

_For example_, we've set the rules for header font-family and font-size in typography.scss. With unit testing, we can quickly check that our SCSS, which looks a little like this:

h1 { 
  font-family: $family-roboto-condensed;
  font-weight: bold;
  ...
  &.b-header {
    font-family: $family-source-sans-pro;
  }
}

when compiled, contains this:

h1 { font-family: 'Roboto Condensed', sans-serif; font-weight: bold; ... }
h1.b-header { font-family: 'Source Sans Pro', sans-serif; font-weight: bold; ... }

[dhruvkb]

Sorry for the random assignments, there are some issues with the design of my GitHub client especially in the positioning of the 'Clear assignees' button.

[kushaggarwal]

Hello @dhruvkb as guided by you I looked upon for working with the library True for unit testing of sass components and tried doing some testing myself too for better understanding
So, can you please guide as how to proceed further with the same in reference to this issue
Thank you

[dhruvkb]

@kushaggarwal I'm actually waiting for @Tanuj22 to setup Storyshots and @silvinabt87 to setup True. I can urge them to speed this up if possible but till then you will have to wait.

[silvinabt87]

I've created an issue (https://github.com/oddbird/true/issues/161) on the repo of oddbird/true because on the True documentation I found the way to test mixins and functions, but not how to test @each.
There are no mixins nor functions on any of the SCSS files to test.
I've already set the tests and they seem to work running 'npm run test'. I will add a draft PR for you to follow up and help me.

[Tanuj22]

@dhruvkb Sorry for the delay. I am having trouble setting up jest. How do I set it up recognise import statements in storybook.test.js file?

[vandnakapoor19]

@dhruvkb Can I work on this?

[dhruvkb]

@Tanuj22 I don't get your question. Since I don't have first-hand experience with 'Storyshots' or 'Automated Visual Testing', I hope you're following this guide.

@vandnakapoor19 yes, please.

[dhruvkb]

PR #179 with True and Jest has been merged and correspondingly the goal of the issue is now to write tests covering the majority of stylesheets in the repository from tokens like colors.scss and spacing.scss to entire components like header.scss.

[kushaggarwal]

Hello @Tanuj22 are you looking up for how to use import statement in storybook.test.js file ?

[Tanuj22]

@dhruvkb Yeah I am. @kushaggarwal Yeah.

[kushaggarwal]

@Tanuj22 I tried unit testing with jest and for that didn't include/import anything in the js file although I did change the type of test in package.json to jest for working with it
Further for testing I just run the command npm run test and it automatically run as per the jest test for the code
Below is an image for reference where in I am checking the sum of two no.s and running a unit test for the same with jest

[jahnvigupta]

@kushaggarwal Which OS are you using?

[silvinabt87]

I created a unit test for color.scss so that you can revise it. The thing is that, according to the advice here, I put the @each rule within a @mixin to be able to test it with True and then ran the code on https://www.sassmeister.com/ to create the expected CSS code. It seems to me that it doesn't make much sense to test that this way, since it will of course show the correct result every time.
This will be more useful, I think, when we have @functions and @mixins to test, but let me know what you think about this and if you have any improvements to my PR.

[dhruvkb]

@silvinabt87 reviewed. You're right that it doesn't make sense to compile entire Sass files and check against CSS, which is why we want to test whether a mixin successfully generates the required classes and not the number of classes it generates.

[silvinabt87]

@dhruvkb should I go on unit testing the other files this way, then? Or do you want to try something different?

[kushaggarwal]

@jahnvigupta I am using MacOS

[dhruvkb]

@silvinabt87 after you make the changes I marked in PR #182, you can use that as a template for other stylesheets.

[kushaggarwal]

Hello @dhruvkb I am working for setting up the the unit testing for other components of vocabulary so shall I also follow the same template as set up by @silvinabt87
Please help
Thank you

[dhruvkb]

Yes @kushaggarwal, but make sure you look at the feedback as well and don't make the same mistakes.

[Tanuj22]

@dhruvkb Please check the above PR. I am getting some errors.

[jahnvigupta]

Do we need interactive testing? If yes, then should I start working on it.

[kushaggarwal]

Hello @dhruvkb
Can you please guide as if for which components can I proceed to do the test for as I believe for some the tests have been written
I have gone through the template in reference to which the tests need to be written
Thank you

[megha070]

@kushaggarwal I guess PRs are raised for below components:
1) color
2) typography
3) Locale
4) Github_corners
5) form
6) Buttons
7) Spacing

[kushaggarwal]

Okay
Thanks @megha070 will start working for the other components

[dhruvkb]

With @jahnvigupta's PR #227 and @silvinabt87's PR #182 we have the infrastructure for both unit tests and visual regression test set up. The problem of writing more tests still remains open.

[kushaggarwal]

Hello @dhruvkb
So for this we have to continue writing unit tests for various components associated with vocabulary right
Please help

[dhruvkb]

Unit tests don't make much sense for components so only mixins and functions should be unit tested. Visual regression tests can and should be written for every component. So pick one or two components max and write tests for them and only move on to other components once those PRs are reviewed and merged.

[kushaggarwal]

Okay so @dhruvkb can I resume the work for other left mixin and function apart from the ones whose PR have been linked above

[jahnvigupta]

@dhruvkb Can you tell for what other elements, snapshot testing needs to be added.

[dhruvkb]

@jahnvigupta whichever is left. We need snapshot testing for all components.

[silvinabt87]

@dhruvkb I'd like to contribute with visual regression tests. @jahnvigupta which components are you working on? So that I can check if there are any left for me to do.

[jahnvigupta]

@dhruvkb I'd like to contribute with visual regression tests. @jahnvigupta which components are you working on? So that I can check if there are any left for me to do.

@silvinabt87 I have done for all components in this repo but we can certainly divide components in
https://github.com/creativecommons/fonts/issues/30
Tell me the components that you would like to work on.

[kgodey]

We're closing this issue because visual regression tests turned out to be flaky. We may reintroduce this functionality in the future but we'll make a new ticket for it.