Fix failing integration tests

[hannalaakso]

Problem

We are seeing the following intergration tests for the Accessible Autocomplete fail as they don't complete within the set time limit of 2000ms:

it('should announce status changes using two alternately updated aria live regions', () => {
      if (isChrome) {
        const flip = browser.$('div#ariaLiveA')
        const flop = browser.$('div#ariaLiveB')

        browser.click(input)
        browser.setValue(input, 'a')
        expect(flip.getText()).to.equal('')
        expect(flop.getText()).to.equal('')
        browser.waitUntil(() => { return flip.getText() !== '' },
          liveRegionWaitTimeMillis,
          'expected the first aria live region to be populated within ' + liveRegionWaitTimeMillis + ' milliseconds'
        )
        browser.addValue(input, 's')
        browser.waitUntil(() => { return (flip.getText() === '' && flop.getText() !== '') },
          liveRegionWaitTimeMillis,
          'expected the first aria live region to be cleared, and the second to be populated within ' +
          liveRegionWaitTimeMillis + ' milliseconds'
        )
        browser.addValue(input, 'h')
        browser.waitUntil(() => { return (flip.getText() !== '' && flop.getText() === '') },
          liveRegionWaitTimeMillis,
          'expected the first aria live region to be populated, and the second to be cleared within ' +
          liveRegionWaitTimeMillis + ' milliseconds'
        )
      }
    })

This happens when they are run through Saucelabs/Selenium. When Saucelabs is not available, the tests are run through Puppeteer which seems to reliably pass.

Solutions explored with @NickColley

1. Remove the timeout limit

Setting timeout on waitUntil is optional.

However removing it throws

Promise was rejected for the following reason: timeout

2. Increase the timeout limit

• Chrome on Saucelabs would intermittently fail within 2000ms; Firefox and IE would always fail
• On 3000ms Firefox and IE11 would still fail.
• On 4000ms IE11 was flaky.
• IE10 needed at least 6000ms to pass, whilst IE9 wouldn't pass even at 10000ms.
• Additionally, Firefox would fail intermittently regardless of the timeout limit set.

3. Limit the browsers the tests run in

As Chrome seemed most reliable to run the tests with, we used the isChrome check to limit the Saucelabs tests only to run in Chrome on Windows 10.

However even then we would occassionally see the tests in Chrome fail within the 2000ms limit.

We increased the limit to 10000ms, at which point no issues were observed.

4. Move the tests to /tests/functional

Saucelabs runs tests in tests/integration. Puppeteer run tests in tests/functional.

Tests on the original PR would have been run through Puppeteer as Saucelabs wasn't available. They seem too flaky to run through Saucelabs because of timer issues so we investigated moving them to /tests/functional.

However we found that trying to create the interactive component for testing was really difficult.

edit for reference: @markhunter27 suggested

We could create a narrower / more focussed functional test that would only assert on the 'bump' state of the component, as that's what's driving which aria live region is targeted. The test could look much the same as the silencing test that's already there (and is also using a timeout). If we call the render() method a couple of times, we should see the bump boolean flip each time. It wouldn't be the full on integration test - like the silencing test it stops short of the markup that comes out of the render() method, but it does test the state generation and assumes we react to the new state correctly within render()

However whilst adding this functional test for the bump state as suggested by @markhunter27 is a good idea, we decided that since the functional tests only run in Chrome it'll be covered by the existing integration tests.

5. Change the tests config

Investigate adding a file that has the same capabilities as tests in /integration (Webdriver?) but is run through Puppeteer rather than Saucelabs. This could be helpful in the future for creating integration tests that don't require Saucelabs.

We weren't sure there was value in this work at this point so didn't much exploration of it.

6. Remove the tests

Saucelabs is flaky when running timer based tests. Remove these tests and leave a code comment to say if the aria-live region code is changed, it should be tested manually.

Proposed solution

Solutions 2 and 3 to keep some test coverage, while also trying to ensure that our PRs don't start failing due to Saucelabs timer issues. With this in mind, keep an eye on the tests on CI going forward.

[36degrees]

Will this mean that in Firefox / IE test output we'll still see 'should announce status changes using two alternately updated aria live regions' appearing to have passed?

I'm wondering if we should either swap the order of these so that we only define the test if isChrome is true, or perhaps even doing something like

(isChrome ? it : it.skip)('should announce status changes using two alternately updated aria live regions', () => {

That way the test would either not appear in the output, or would be explicitly listed as having been skipped.

Thoughts?

[NickColley]

Yeah this seems worth having a poke, but if it's too difficult this follows the same pattern elsewhere in the test suite when running into browser specific issues.

[hannalaakso]

@36degrees We've wrapped the tests in a conditional block. Your above code seems to work too but the linter was complaining about it, plus a block is just simpler.

[36degrees]

Is this change still needed if we're only testing in Chrome?

[NickColley]

Yes unfortunately :(

[36degrees]

It might be worth updating the commit message, as at the minute it reads as though we're 'just' doing option 3, when it appears we're really doing both 2 and 3?

[hannalaakso]

Yes good point @36degrees, I'll update the commit message once the tests have passed once.