Image-sequencer: Writing more Jest UI tests and making UI more stable

Created on 18 Dec 2019  Â·  27Comments  Â·  Source: publiclab/image-sequencer

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 )

UI help wanted high-priority planning stability testing

Most helpful comment

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 notifications@github.com 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
.

All 27 comments

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.

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 notifications@github.com 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
.

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

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 notifications@github.com 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
.

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.

I have published this on gci dashboard

Thanks Keshav

@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.

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.

I would like to work on clear step test, Thanks

@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.

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

@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

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.

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

image
@keshav234156 we are talking about this button right?

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

image
this one then?

@pythongiant Yes

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.

@pythongiant Are you working on the delete step?

Nope @ataata107

Ok I will start working on it

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!

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

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?

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

Others we want to copy in here?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

harshkhandeparkar picture harshkhandeparkar  Â·  4Comments

jywarren picture jywarren  Â·  4Comments

jywarren picture jywarren  Â·  4Comments

VladimirMikulic picture VladimirMikulic  Â·  3Comments

harshkhandeparkar picture harshkhandeparkar  Â·  5Comments