Image-sequencer: Writing more UI tests (with guidance)

[jywarren]

We need more UI tests to protect and stabilize the UI behaviors of the demo, so that we know when refactors and new features break things. This will build overall stability in the system and let us do more advanced work without chasing bugs. Bugs are natural! No worries! But we can catch them before they get published using more tests.

Our UI tests can be found here: https://github.com/publiclab/image-sequencer/tree/main/test/ui/spec

Instead of testing core functions with unit tests, they set up an HTML environment and simulate someone actually using the buttons and UI.

So in one of these test files, or a new one, we should be able to begin interacting with a full Sequencer UI, by selecting things in the Add Modules select, clicking buttons with $('.btn .uniqueClass').trigger('click'); (or something like that) and then using assertions to test that things really happened, something like this:

    expect($('.step').length).toBe(10); // number of steps shown has changed

This way, we can have more confidence in the UI not breaking. We'd love help with this! I know @Harshithpabbati asked a bit about how this might work. Want to try it out?

Once we have one simple test like this, we can use it as a model to add more like it, and get to higher test coverage of our UI code!

[jywarren]

For example, we should be able to input settings using something like $('.step .step-2 input.brightness').val(24) --

But first, we'll need to add more unique CSS classnames so that we can make calls like that in jQuery. Right now, they're too generic:

<div class="panel-body cal collapse in">
            <div class="row step">
                <div class="col-md-4 details">
                  <h3><span class="load-spin" style="display:none;"><i class="fa fa-circle-o-notch fa-spin"></i></span></h3>
                  <div class="cal collapse in"><p><i>Change the contrast of the image by given value</i></p></div>
                <div class="row" name="contrast">
                    <div class="det cal collapse in">
                           <label for="contrast">contrast for the new image, typically -100 to 100</label>
                           <input class="form-control target" type="range" name="contrast" value="70" placeholder="" min="-100" max="100" step="1">

[VibhorCodecianGupta]

I'd like to take this up. This would be kind of a quest, adding UI tests as we go deeper into implementing more features for IS. Would like to start with some basic tests

[jywarren]

:-) great! And love the idea of a test-quest! You might like this fun
project i once did:

http://pxlqst.com/test (https://github.com/jywarren/pxlqst)

On Mon, Apr 8, 2019 at 7:29 PM Vibhor Gupta [e-mail]
wrote:

I'd like to take this up. This would be kind of a quest, adding UI tests
as we go deeper into implementing more features for IS. Would like to start
with some basic tests

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/image-sequencer/issues/1000#issuecomment-481044734,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ-Jioj6j7ws-PraIvb1iYK8g4AgLks5ve9C8gaJpZM4cjH8N
.

[VibhorCodecianGupta]

Nice! I'll get on with improving some tests I already wrote while setting up the UI testing harness.
Rendering of buttons and steps should be a nice starting point.

Also, apologies for shamelessly plugging in my proposal, but trying to make it as comprehensive as possible. What are the chances I'll get reviewed, should I submit one in a couple of hours? It's early morning here, trying to churn it out as fast as I can. Thanks!

@jywarren

[jywarren]

It's very close to the deadline - we will read it for sure, but not sure if
it'll be in time to file a revision! But thank you!!!

On Mon, Apr 8, 2019 at 7:50 PM Vibhor Gupta [e-mail]
wrote:

Nice! I'll get on with improving some tests I already wrote while setting
up the UI testing harness.
Rendering of buttons and steps should be a nice starting point.

Also, apologies for shamelessly plugging in my proposal, but trying to
make it as comprehensive as possible. What are the chances I'll get
reviewed, should I submit one in a couple of hours? Thanks!

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/image-sequencer/issues/1000#issuecomment-481048628,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ-Po1uL4LEhfllLZklngs5YuBRbhks5ve9W0gaJpZM4cjH8N
.

[harshkhandeparkar]

Hi. Can someone help with the testing of the $step method? See #833 for reference

@publiclab/is-reviewers @jywarren

[Divy123]

Hi Harsh! I want to work on UI testing and would love to look into!!

[VibhorCodecianGupta]

@jywarren again, apologies for the plug, I sent over the commentable link to my proposal to you on Gitter. My proposal on publiclab.org says I'll be notified via mail once my proposal gets published. It is way too late for reviews, but the link is on Gitter for your reference. Thanks!

[blurry-x-face]

I would love to work on this issue

[harshkhandeparkar]

sure go ahead

[Divy123]

Yes please go ahead!

I would love to collaborate on this.

On Fri 30 Aug, 2019, 10:26 PM Rishabh Shukla, [e-mail]
wrote:

I would love to work on this issue

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/image-sequencer/issues/1000?email_source=notifications&email_token=AHOHJL6RFD4HPNUUM7BXUWDQHFGKPA5CNFSM4HEMP4G2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5SGOIA#issuecomment-526673696,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHOHJL7KO2SQCM4QJ7YDG3TQHFGKPANCNFSM4HEMP4GQ
.

[jywarren]

Much ❤️ to you all!!!

Shall we make a checklist of UI actions that could be tested for?

• [x] clicking Add Step and confirming a new box appears, and a new image appears - https://github.com/publiclab/image-sequencer/blob/b7eb9cfc45e3569a737adfc9a8d3e4dc1ae681e3/test/ui-2/test/clear_and_delete_step.test.js#L13-L56
• [x] Add Step for various modules - https://github.com/publiclab/image-sequencer/blob/b7eb9cfc45e3569a737adfc9a8d3e4dc1ae681e3/test/ui-2/test/clear_and_delete_step.test.js#L13-L56
• [x] Clicking Delete Step removes the image https://github.com/publiclab/image-sequencer/blob/main/test/ui-2/test/clear_and_delete_step.test.js#L58
• [ ] Clicking Insert Step adds a new box, and modifies the image

These could be largely independent from the UI implementation as long as the add step, delete step, insert step elements have standard classnames!

(update: checked boxes above)

[jywarren]

Also updated location of existing UI tests! https://github.com/publiclab/image-sequencer/tree/main/test/ui/spec

[jywarren]

Here is the most likely section for the listed tests above!

https://github.com/publiclab/image-sequencer/blob/3741d679bd34812efb29d256bb4e033340d086e9/test/ui/spec/defaultHtmlSequencerUi.spec.js#L27-L41

Note that we are not yet testing click events and such. I think the tests are just really minimal stubs, confirming that the UI was set up, but not testing that it works, is that right?

Also:

• [ ] steps included via URL hash are correctly parsed and added (many variants of this, see https://github.com/publiclab/image-sequencer/blob/257113a948a2b8c2026bc07caffa9445fcde51b8/test/ui/spec/urlHash.spec.js#L1)

[Divy123]

As I discussed with Pranshu, he suggested that testing with Jest along with puppeteer is a good way to proceed here.
What do you think @jywarren , @blurry-x-face ?

[jywarren]

Hi! I'm not against a weighing of pros/cons to switching test libraries, but is there a problem with the current test setup that you're interested in addressing by switching?

[jywarren]

Gaah! I accidentally committed to main branch here, i'm sorry:

https://github.com/publiclab/image-sequencer/commit/fb452f7788b7f0641fe1419f649ff5f5ba0f3be9

However, you can see a very clear UI test that is only 3 lines, for the quick selector! cc @harshithpabbati @Divy123 @rexagod @blurry-x-face

[jywarren]

I apologize for the mistake, i'll revert it as soon as it fails, but will open it in a new PR.

[jywarren]

Reverted, now working on it at: #1242

To address #1238, which Harsh identified, it would be something like:

  it('allows changing brightness value using input and Apply button', function() {
    $("[data-value='brightness']").click();
    var brightnessImage1 = $('.step:last img')[0].src;
    $('.step:last input.form-control').val('25%'); // change the value
    $('.step:last .btn-save').click(); // apply the change
    var brightnessImage2 = $('.step:last img')[0].src;
    expect(brightnessImage1).not.toEqual(brightnessImage2);
  });

[jywarren]

Does this makes sense to you all? cc @publiclab/is-reviewers Thanks!

[blurry-x-face]

Hi! I'm not against a weighing of pros/cons to switching test libraries, but is there a problem with the current test setup that you're interested in addressing by switching?

Actually jest is a more maintained testing library but I will look through jasmine.

Thanks @jywarren
cc: @Divy123

[vaibhavmatta]

@jywarren I am claiming it now. Let us work on it together.! The UI needs to be stable. Will make a PR for this super soon.
Thanks

[jywarren]

Closing this in favor of #1369 now!