One skipped, one slow front-end test

[lyzadanger]

I've noticed that the LMS front-end test suite has one skipped test and one test that complains about being slow on every test run (locally, at least). I wanted to note them down for follow-up.

• Skipped test: in the StudentSelector component
• Slow test: reported as

Chrome Headless 91.0.4472.114 (Mac OS 10.15.7) SLOW 1.039 secs: BasicLTILaunchApp should pass a11y checks

[lyzadanger]

I did briefly look into this in this sprint. What I found was:

• The skipped test is an accessibility test in the StudentSelector component. It is skipped because it will fail because the <select> in that component does not provide a <label>. I suggest we look at whether it's feasible to add a <label> to that <select>.
• The slow test is an accessibility test in BasicLTILaunchApp that takes between 0.5 and 1 second to run. That is indeed very slow. That's a complex component with a fair amount of test setup, and I didn't see an immediate fix. It certainly bears scrutiny and should be returned to when we have a moment to spare.

[robertknight]

I suggest we look at whether it's feasible to add a <label> to that <select>.

Visually the purpose of the widget looks quite obvious to me:

It seems that we could add an aria-label for assistive tech users and be done with it. I've verified with VoiceOver that the label gets read. One thing I'm not certain about is whether the label should be a command ("Select student") or status ("Selected student"). Thoughts @lyzadanger ?

[lyzadanger]

One thing I'm not certain about is whether the label should be a command ("Select student") or status ("Selected student").

I think because it is an interactive element the command makes sense. (Think of a label for a typical select).

This is totally something we should do. I can take care of it.

[robertknight]

Thanks for offering @lyzadanger. I had just created https://github.com/hypothesis/lms/pull/4111 while testing whether the current version of axe-core saw any other issues. Feel free to revise that if you think it needs any changes.

[robertknight]

The skipped test has been resolved. That just leaves the slow test. Since there is only one, and it takes one second to run, I think we could afford to just disable the warning for now, for the specific test.