image-sequencer icon indicating copy to clipboard operation
image-sequencer copied to clipboard

Writing more Jest UI tests and making UI more stable

Open keshav234156 opened this issue 6 years ago • 28 comments

With #1366 we were able to set up a basic browser-based UI test and it works flawlessly. So lets build more tests and make the UI stable. Test yet to be built:

  • [x] (Reserved For GCI ) Clicking add step adds a new box and a different image(#1387 )
  • [ ] Clicking clear step button clear all the step
  • [x] Clicking Delete Step removes the image(Fixed within #1499 )
  • [ ] (Reserved For GCI ) Select a module to have all the modules with the correct name(we faced issues when modules have which is undefined in the past ).
  • [ ] Insert step works as expected.
  • [ ] Input types(range,text etc) of all the modules are correct- we need to fix this before we can write test on it
  • [ ] add a UI test for the test which check that all the modules the default options are displayed(#1510 )
  • [ ] insert step button tested https://github.com/publiclab/image-sequencer/issues/1000#issuecomment-526683755
  • [ ] https://github.com/publiclab/image-sequencer/issues/1238#issuecomment-527008195 - via @HarshKhandeparkar in #1238 - If you click on the step collapse btn and if you have multiple steps, clicking on the load-image step collapses something totally different. After some time, all the collapse btns break.

keshav234156 avatar Dec 18 '19 19:12 keshav234156

cc:-@jywarren @HarshKhandeparkar we can add more tests as we go along the way but these are most basic set of tests that are needed.

keshav234156 avatar Dec 18 '19 19:12 keshav234156

This is FANTASTIC!!!! Thanks!!!

If we write the tests generically enough, using simple classnames, then the new UI Harsh has been working on could be run against the same set of tests! This would be amazing. Let's keep this in mind and Harsh, maybe try to get your classnames to match so that your UI work passes these tests too?

On Wed, Dec 18, 2019, 2:13 PM keshav234156 [email protected] wrote:

cc:-@jywarren https://github.com/jywarren @HarshKhandeparkar https://github.com/HarshKhandeparkar we can add more tests as we go along the way but these are most basic set of tests that are needed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/image-sequencer/issues/1369?email_source=notifications&email_token=AAAF6J4ISDEMOIBAWUERI3DQZJY5TA5CNFSM4J4PXBKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHHFLFI#issuecomment-567170453, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J7B3UZAQNPPABOUJKDQZJY5TANCNFSM4J4PXBKA .

jywarren avatar Dec 18 '19 19:12 jywarren

I am new here and would love to work on this.

ataata107 avatar Dec 18 '19 21:12 ataata107

we'd love your help!!! Which would you like to try? Go ahead and open a pull request with the new test; start with something simple and then keep building on it!

On Wed, Dec 18, 2019, 4:38 PM Shazeb Ata [email protected] wrote:

I am new here and would love to work on this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/image-sequencer/issues/1369?email_source=notifications&email_token=AAAF6J4N3INWM5SIS7EB7ZTQZKJ7DA5CNFSM4J4PXBKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHHSSPY#issuecomment-567224639, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J7HEFRWUMWOCL7ARY3QZKJ7DANCNFSM4J4PXBKA .

jywarren avatar Dec 18 '19 21:12 jywarren

I will start with the basic ones like clear step button and remove step button and would start building from it. Thanks for the opportunity.

ataata107 avatar Dec 18 '19 21:12 ataata107

I have published this on gci dashboard

SidharthBansal avatar Dec 19 '19 02:12 SidharthBansal

Thanks Keshav

SidharthBansal avatar Dec 19 '19 02:12 SidharthBansal

@jywarren sure. The new UI will just be less messy and it is coming together really well. I think I do not need to add the E2E tests which would require headless browsers. I will just add the unit tests with perhaps a virtual DOM. Is that ok? We can use the tests we are developing here, there.

We won't have to migrate anything, we can just copy the dist from there and use grunt here.

harshkhandeparkar avatar Dec 19 '19 17:12 harshkhandeparkar

describe('Add step', () => {
  test('length is increased', async () => {
    	await page.waitForSelector('.step');
    const previousLength = await page.evaluate(() => document.querySelectorAll('.step').length);
    await page.click('[data-value=\'brightness\']');
    const previousLength1 = await page.evaluate(() => document.querySelectorAll('.step').length);
    expect(previousLength).toBe(1);
    expect(previousLength1).toBe(2);
  }, timeout);
});

Doesnt this check if a new step is added? @keshav234156 i am just a beginner sorry if i am missing something.

pythongiant avatar Dec 22 '19 04:12 pythongiant

I would like to work on clear step test, Thanks

pythongiant avatar Dec 22 '19 04:12 pythongiant

@pythongiant This piece of code tests that when the radio button is clicked a new step is added and length is increased . In the GCI task, you have test when we select a value from the dropdown and click ADD STEP a new is added and src of the previous step is different from src of the present step. @ataata107 is already working on the Clear step.

keshav234156 avatar Dec 22 '19 04:12 keshav234156

Thanks for the explaination @keshav234156 i must have skipped @ataata107 comment by mistake. Sorry bout that. Can i work on delete one?

pythongiant avatar Dec 22 '19 07:12 pythongiant

@pythongiant It's easy.I will help you here . with $("[data-value='blur']").click() You would be able to set dropdown value to blur with $("#add-step-btn").click() you can click ADD STEP button

Now just check that length has increased as done above and check src of both step is different

keshav234156 avatar Dec 22 '19 07:12 keshav234156

I am currently working on the 4th one (Select a module dropdown having the correct names). I claimed the corresponding task on the GCI dashboard.

rcya1 avatar Dec 22 '19 23:12 rcya1

@pythongiant Are you still stuck? Feel free to ask any doubt related to the task. @Ryan10145 great!!!

keshav234156 avatar Dec 23 '19 00:12 keshav234156

image @keshav234156 we are talking about this button right?

pythongiant avatar Dec 23 '19 03:12 pythongiant

@pythongiant No,This is Add step button of insert Button.We are talking of ADD STEPbutton at the bottom.

keshav234156 avatar Dec 23 '19 04:12 keshav234156

image this one then?

pythongiant avatar Dec 23 '19 04:12 pythongiant

@pythongiant Yes

keshav234156 avatar Dec 23 '19 04:12 keshav234156

Due to holiday celebrations and other commitments, I wasn't able to work on this for the last few days like I thought I would be able to. I also won't be able to work on it in the near future, so I am giving up the task and leaving it up for anyone to claim. Sorry for the unfortunate news.

rcya1 avatar Dec 26 '19 22:12 rcya1

@pythongiant Are you working on the delete step?

ataata107 avatar Dec 27 '19 21:12 ataata107

Nope @ataata107

pythongiant avatar Dec 28 '19 18:12 pythongiant

Ok I will start working on it

ataata107 avatar Dec 29 '19 18:12 ataata107

Hi all, this is coming along really well and I wanted to thank everyone for your help. This is doing a huge amount to stabilize our project!

jywarren avatar Jan 27 '20 22:01 jywarren

Noting Apply button test is ready in #1579, and maybe we need to update what still needs tests?

jywarren avatar Jul 12 '20 19:07 jywarren

One could be - a test of which steps get refreshed when we modify things. To ensure that only those tests that are "after" a change get refreshed; i think i've seen some unnecessary step reruns in manual testing.

Others we could test for?

jywarren avatar Jul 12 '20 19:07 jywarren

Noting a few that've been observed and we should test for in Jest:

  • insert step button tested https://github.com/publiclab/image-sequencer/issues/1000#issuecomment-526683755
  • https://github.com/publiclab/image-sequencer/issues/1238#issuecomment-527008195 - via @HarshKhandeparkar in #1238 - If you click on the step collapse btn and if you have multiple steps, clicking on the load-image step collapses something totally different. After some time, all the collapse btns break.

Others we want to copy in here?

jywarren avatar Jul 12 '20 19:07 jywarren

Maybe we should check in on whether some of these tests have been written and could be checked off?

jywarren avatar Sep 14 '21 14:09 jywarren