search

accordproject / web-components

React Components for Accord Project
Apache License 2.0
121 stars 103 forks source link

User Interaction Testing #14

Open jolanglinais opened 4 years ago

jolanglinais commented 4 years ago

This issue will track the tests which need to be implemented in order to increase stability of this codebase. Please feel free to add suggestions.

MASTER_MARK_LIST: (@riverceebontu)

MASTER_BLOCK_TRANSFORM:

MASTER_HISTORY_CHECK:

Toolbar

Headings (@sanketshevkar)

Marks (@Cronus1007)

Bold

Rendering

Breaks

Actions

Shortcuts

sanketshevkar commented 3 years ago

May I start working on this?

sanketshevkar commented 3 years ago

doubt @irmerk I'm not able to find the code for this test.

jolanglinais commented 3 years ago

@sanketshevkar none of these tests currently exist. The Issue is for creating them.

sanketshevkar commented 3 years ago

@sanketshevkar none of these tests currently exist. The Issue is for creating them.

No problem, the checkbox was checked, that's why I thought so.

jolanglinais commented 3 years ago

OH! Yeah I don't know why that is, that must've been a mistake on my part sorry about that.

sanketshevkar commented 3 years ago

I'm writing the interaction tests using cypress. The storybook documentation also suggested using enzyme or cypress for interaction testing. Also setting up enzyme within storyshots was a bit complicated so decided to use cypress which is working fine. I did some dummy tests with the web-components storybook and seemed to workout, I'll share the video by tomorrow. Just wanted to know whether this approach would be right?

jolanglinais commented 3 years ago

This approach doesn't strike me as wrong so far, so yeah let's see how this progress goes. Storybook suggesting Cypress is a good sign that it would be a good approach.

sanketshevkar commented 3 years ago

https://www.loom.com/share/b128d0a885694420bf4d55edda57ab73 Video for cypress test.

  1. Places cursor inside the paragraph:

    This is text. This is italic text. This is bold text. This is a link. This is inline code.

  2. Changes it to Header 2.
  3. Undo
  4. Redo
sanketshevkar commented 3 years ago

@irmerk what is the difference between

Selecting a paragraph and changing to header 1, 2, 3

Switching paragraph to header 1, 2, 3

jolanglinais commented 3 years ago

Very cool. I haven't used Cypress before. It looks like we'll want to set up an OSS plan with them (instructions) and implement this in GitHub Actions for this repo.

These two you mentioned may end up being the same. In my mind I wanted to distinguish the difference between highlighting an entire paragraph with the cursor, placing the cursor inside the paragraph, highlighting a word, and highlighting the paragraph with a triple click.

jolanglinais commented 3 years ago

I put in a request to Cypress. If approved, Cypress will provide AP with a plan that allows unlimited test recordings and up to 5 users. All projects we create in the Cypress Dashboard will be public. Data will be retained for 90 days. Our organization will have access to all the features the Cypress Dashboard provides, including parallelization.

CC: @jeromesimeon @DianaLease @dselman @mttrbrts @accordproject/maintainers-ui @accordproject/tsc @accordproject/contributors

jolanglinais commented 3 years ago

I think just continue on @sanketshevkar, I'm opening another Issue to track setting up Cypress for this repo.

sanketshevkar commented 3 years ago

@irmerk That sounds great. Should I create a pr for cypress setup and then start working on the tests?

jolanglinais commented 3 years ago

What would that involve @sanketshevkar?

sanketshevkar commented 3 years ago

Basically has two parts:

  1. Setting up cypress on your local machine.
  2. Setting up cypress within your project, which is a simple npm install command, update scripts in package.json, after running the test for the first time it sets up a cypress folder with some sample tests. I'll delete the example tests within the cypress folder and create a file for markdown-editor test.
jolanglinais commented 3 years ago

Yeah I think that works

sanketshevkar commented 3 years ago

@irmerk I've made a pull request with the cypress setup.

Cronus1007 commented 3 years ago

@irmerk @sanketshevkar Cypress testing only deals with the automated testing software for e2e whereas there is a need to lay down the setup for unit testing as well.

jolanglinais commented 3 years ago

What do you propose @Cronus1007?

Cronus1007 commented 3 years ago

@irmerk The Jest will be the best open source tool for unit testing of React Web Apps but I highly prefer a combined mocha and chai for the testing of js applications. Plus can start the work for cypress once #282 gets merged.

mttrbrts commented 3 years ago

We should follow the precedent set in ui-concerto that uses Jest and react-testing-library for unit tests.

Cronus1007 commented 3 years ago

This issue will track the tests which need to be implemented in order to increase stability of this codebase. Please feel free to add suggestions.

Hotkeys in markdown-editor have not been added i guess.

jolanglinais commented 3 years ago

Now that #282 is finished, this Issue becomes a lot more accessible. I'm going to mark it as unassigned because this is probably the single most important Issue in this repo currently and I want to leave it open for as many people to work on it as possible.

sanketshevkar commented 3 years ago

Now that #282 is finished, this Issue becomes a lot more accessible. I'm going to mark it as unassigned because this is probably the single most important Issue in this repo currently and I want to leave it open for as many people to work on it as possible.

Works for me 👍.

sanketshevkar commented 3 years ago

@irmerk I wanted to discuss the structure of the tests. Basically, there are two ways we can do it:

  1. mount the component and test the interactions.
  2. load the storybook using the localhost url and test the component interactions.

The first one is suggested by cypress docs and the second one by storybook docs, which one would be preferable? I've tried using the second method and it was working.

mttrbrts commented 3 years ago

@sanketshevkar Let's be pragmatic, the second option works for you, and we have a sample here: https://github.com/accordproject/web-components/blob/master/packages/storybook/cypress/storybook/storybook.spec.js

Let's go with 2.

One downside is that it requires a contributor to run tests for the storybook package when only making changes to an individual component. However, we can probably mitigate that with some test filtering and good docs.

sanketshevkar commented 3 years ago

@sanketshevkar Let's be pragmatic, the second option works for you, and we have a sample here: https://github.com/accordproject/web-components/blob/master/packages/storybook/cypress/storybook/storybook.spec.js

Let's go with 2.

One downside is that it requires a contributor to run tests for the storybook package when only making changes to an individual component. However, we can probably mitigate that with some test filtering and good docs.

Okay, that sounds cool. I'll move ahead with this.

Cronus1007 commented 3 years ago

@sanketshevkar Since the issue is very large so lets divide issue.

sanketshevkar commented 3 years ago

@sanketshevkar Since the issue is very large so lets divide issue.

I've been anyways unassigned from this one. It's an open issue now. I've started with testing headings. If you want you can join in for that or pick up any other part you want.

Cronus1007 commented 3 years ago

I've been anyways unassigned from this one. It's an open issue now. I've started with testing headings. If you want you can join in for that or pick up any other part you want.

Okk So I will pick up the Marks part then.

jolanglinais commented 3 years ago

Yeah maybe this isn't the best way, but I'm thinking call out which one you want to work on and we can put your name next to it in the original post. So I'll put @sanketshevkar on headings and @Cronus1007 on marks.

sanketshevkar commented 1 year ago

@mttrbrts @irmerk is this still needed/open? If yes, I can pick this up again 😄