chore: Switch regression test to GitHub Actions

[nschonni]

I played around with keeping the parallel job splitting by using letter ranges in a separate branch, but found that this was the most straightforward.
This uses @spectranaut script for limiting the runs to changed files, and adds the GitHub Action path filtering so the job is also only triggered if those files are touched.

@spectranaut should the full suite run on merges to the main branch?

[mcking65]

@nschonni asked:

@spectranaut should the full suite run on merges to the main branch?

No, we are limiting for performance reasons. It takes too long to run the full suite for every merge to the main branch.

This is probably unrelated, but I wonder if we need to test both the branch and the PR when merging from a branch to main. Isn't it sufficient to test the PR?

[nschonni]

I can turn off the branch testing for pushes to the default branch. I had left the push part on, because I thought it might still be useful for people working on their forks, or for testing on feature branches.

From https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-ignoring-branches-and-tags it would mean adding the following to the existing push section

    branches-ignore:
      - master

[nschonni]

PS: I found it only took around 9 minutes to run the full set of regressions https://github.com/nschonni/aria-practices/runs/1076320104?check_suite_focus=true when I just called the npm run regression in my first take on this

[spectranaut]

cool that the tests ran so fast! Ava can parallelize in two ways -- by VM or by process. You took out the by VM parallelization we were using but we can revisit if the tests become slow in the future -- 9 mins is similar to what we had after the VM parallelization.

However, see my comment about flaky tests -- Ava is probably using process parallelization right now.

[mcking65]

@spectranaut you're comfortable merging without further testing for flakiness?

[nschonni]

Do you also want me to filter it out from running on merges to master?

[spectranaut]

Actually @nschonni can you trigger a bunch of runs on your fork to test for flakiness? Will pushing an empty commit trigger the github action?

Also we might as well filter out running merges to master, as you suggested!

[nschonni]

@spectranaut I've added you to my fork. You can rerun any of the jobs again by hitting the "Re-run jobs" button on the top right of any Action run https://github.com/nschonni/aria-practices/runs/1077220667?check_suite_focus=true

[spectranaut]

@nschonni awesome but unfortunately all of the tests are skipped in the PR (by the bash script) do re-running it won't show anything. We need to edit the bash script for a test/index.js or test/util/* file to trigger all the test to run. I can push something (called "WIP" we can remove like your other tests) and then rerun

[nschonni]

Ah, here is an older workflow that just called the npm run regression as well https://github.com/nschonni/aria-practices/runs/1076320104?check_suite_focus=true

[spectranaut]

Hmm I just ran all tests twice and both times the same test failed (here and here) -- it's a test that opens a link, it could be a time out. We could try extending the time out or switching to concurrency = 1?

Edit: three times and every time the same failure!

[nschonni]

Arg, green here, but red over here https://github.com/nschonni/aria-practices/runs/1088609115?check_suite_focus=true

[nschonni]

@spectranaut I added back your parallelization of the job (and increased it to 5, because why not :)), but I'm still getting consistent failures on not ok 113 - treeview_treeview-2a › treeview/treeview-2/treeview-2a.html [data-test-id="key-enter-or-space"]: Key enter opens folder and activates link

Edit: consistent on this PR, but sometimes till green on the fork run

[nschonni]

@spectranaut I'm still getting random failures in some treeview tests, but I also hit the same thing with Travis on another PR, so I don't think this is introducing anything new.
I bumped the parallel up to 10 jobs just to see if it helped, but I can reduce it again if you want. Also turned off "fast-fail" so all the jobs will finish even if one fails, and that one job can just be restarted (if you have permission),

Should I rebase off the the last commit so this can land?

[spectranaut]

@nschonni that all sounds good to me. I think we can fix the treeview tests (I have some ideas!) but I just haven't had a moment to look into it.

[nschonni]

Thanks @spectranaut, I forgot to mention I also increased the timeout to 10000 to see if it helped, but I can put it back to 1000

[spectranaut]

@nschonni did increasing the jobs to 10 cause a noticeable speedup? because I think we might as well decrease it to 5 simply because it might make looking through failures a bit annoying (if you have to scroll through/click through some many different jobs to see them all).

Also are you saying that increasing the timeout from 1000 to 10000 didn't actually help prevent the treeview failures? that's pretty crazy. I looked at your fork but didn't see where I could find them. Can you link me a failure with the 10000 or is it too hard to find..? I could alternatively run some tests in my own branch.

[nschonni]

I looked at your fork but didn't see where I could find them

Ah , sorry, I found out I could delete old Action results so I cleaned up the various names I had used for the job name that were cluttering up left side bar. I think it's more likely a "flaky" test where Ava might be missing some sort of ready even rather than a real timeout. Seems most of the failures I was seeing were around the Treeview tests, which can be finicky I guess :)

I'll change the parallelization back to 5

[nschonni]

@spectranaut back to 5 in parallel now

[spectranaut]

@mcking65 Ok! I think this is good to merge :)

[nschonni]

Thanks @spectranaut, think you might need to change your review from "Request Changes"

[spectranaut]

Definitely approving this PR! I tested to see if I could see the flaky treeview tests.. and I can't any more: bocoup#32 (note: on that branch there are travis build failings you have to click the checks to see the regression pass every time)

The tests all rely on reaching wikipedia, so maybe there was some github checks infrastructure internet problem when we first tested this? Not sure. Anyway stoked for this change!

[nschonni]

Ah, interesting. Maybe there is some rate-limiting going on. I know GitHub itself can sometimes return 429 status' when limits are hit

[nschonni]

@mcking65 I've rebased this one now that the Stylelint PR landed

[mcking65]

@nschonni thank you for all your work on this!!

[nschonni]

@spectranaut was checking out link checkers and it looks like wikipedia is returning 429 responses on a bunch of those links. Might be work trying to find a different site for the links (if that makes sense)

[spectranaut]

@spectranaut was checking out link checkers and it looks like wikipedia is returning 429 responses on a bunch of those links. Might be work trying to find a different site for the links (if that makes sense)

Hmm I see... Maybe we should have a policy of only redirecting to internal websites for examples to make sure the examples remain easy to test. Or we could just add some kind artificial slow down when testing tests that refer to external websites. For now lets just keep an eye on it and address it if it causes failures, and address it then?