test for jest conversion

[jywarren]

Working from these for triggering the event without jQuery:

• https://javascript.info/dispatch-events
• https://stackoverflow.com/a/46815122/1116657

Sourcing syntax from this: https://github.com/publiclab/PublicLab.Editor/blob/main/test/ui-testing/bold.test.js

Trying to convert this Jasmine test:

PublicLab.Editor/spec/javascripts/button_spec.js

Lines 13 to 21 in f8e1edb

	it("Publish button is enabled", function() {
	// Check initially that Publish button is disabled.
	expect($('.ple-publish').prop('disabled')).toBe(true);
	// Add title.
	$('.ple-module-title input').val('A title');
	$('.ple-module-title input').keydown();
	// Check final state of Publish button.
	expect($('.ple-publish').prop('disabled')).toBe(false);
	});

[gitpod-io]

[jywarren]

What's odd is that I thought we use the /Bootstrap class/ disabled to set the disabled state on this. I don't know how that interacts with the HTML attribute -- the difference is:

<i class="disabled"></i> (Bootstrap CSS)
<i disabled></i> (HTML)

So we may need to disambiguate how this is working, possibly by manually testing the jquery $('.ple-publish').prop('disabled') vs. just el.disabled.

[jywarren]

OK - so i think that's right:

  ● Publish button › Publish button is enabled

    expect(received).toBe(expected) // Object.is equality

    Expected: true
    Received: false

      10 |   test('Publish button is enabled', async () => {
      11 |     // Check initially that Publish button is disabled.    
    > 12 |     expect(await page.evaluate(() => document.querySelector('.ple-textarea').disabled)).toBe(true);
         |                                                                                         ^
      13 |     // Add title.
      14 |     await page.evaluate(() => {
      15 |       document.querySelector('.ple-module-title input').value = 'A title'

We should try this: document.querySelector('.ple-publish').style.includes('disabled'));

oh wait. First we should be checking ple-publish and not ple-textarea! Doh!

[jywarren]

That worked! @NARUDESIGNS 🎉

So, maybe what we should do is to look closely at the old test vs. this new one. And to come up with a little bit of helpful documentation (even just a short checklist) for how to do this kind of test conversion successfully. It looks like a combination of:

• .evaluate()
• adding async
• switching to non-jQuery functions

Does that sound right?

[jywarren]

Would you like to try that for an additional test file now? I'll merge this. I hope this'll get us moving now! Thanks Paul, for your patience!!!

[jywarren]

Ah, and we should add to the checklist - removing the tests from the Jasmine files. So when we finish all conversions, we'll have no more Jasmine tests!

[NARUDESIGNS]

Wow awesome!
I took some time to rest my head after our meeting. I can't wait to see those tests turn green!!!
I'd have a look at it tomorrow, it's 11pm here right now.
Thank you @jywarren you are amazing! 🔥 🔥 🔥

[NARUDESIGNS]

Hi @jywarren I noticed you didn't add the timeout variable here. Is this part of the new change or is it something you missed out?

[jywarren]

I think I was able to set it globally on the first few lines and so didn't need to set it per-test.

[jywarren]

Ah, interesting, actually on checking, I am not setting the timeout. I think that means we could remove that line, actually.

For reference, I'm stuck on a related issue (going past the timeout limit in this case) in this test publiclab/image-sequencer#2026

https://github.com/publiclab/image-sequencer/blob/58eb3ef6b1caba88aaf217d93ec085a14765d8aa/test/ui-2/test/OverlayInput.test.js#L1-L12

That actually is globally setting timeout. But here I think i just removed it and for the purposes of our test it wasn't needed. I think the default timeout is 5 or 10 seconds? So i guess that's enough for this.