Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Occasional regression test timeout when waiting for link to load #1594

Closed
spectranaut opened this issue Oct 29, 2020 · 6 comments · Fixed by #1600
Closed

Occasional regression test timeout when waiting for link to load #1594

spectranaut opened this issue Oct 29, 2020 · 6 comments · Fixed by #1600
Labels
enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation
Milestone

Comments

@spectranaut
Copy link
Contributor

We have a few times seen failures related to timeouts when trying to test functionality in an APG example that results in following a link to an external page.

Here is an example: bocoup#43 You can see if fail on the first commit and pass on the next commit. (The commits just include debugging statements in them). I originally saw this failure in #1424.

When switching from travis to github checks, we saw some failures to follow through on links and discussed options: #1515

One option is to have a policy of not having external links in our example widgets.

@mcking65
Copy link
Contributor

@spectranaut
Regression 2 is consistently failing. It failed in #1532 and #1533. It appeared to be time outs in both cases. I am assuming because of this external link issue.

I guess I was able to merge both of those PRs even with the failures because we haven't yet changed the branch policies (#1548). Is that right @nschonni?

I have no problem with a no external link policy in examples. That does seem like it would be a bit of work to clean up. There are several examples with external links.

How to best clean up the external links is a good question. In some cases, like the disclosure nav example and new tree nav example that is in progress, we use the link to load some content. In other places, like the grid of search results, the links are just placeholders and don't have to do anything. Could we just have them all popup an alert that says that if this were a real link it would go some place? Or, should they just do nothing at all? @jongund, what do you think?

Would a stopgap be to let those tests fail until we get the external links removed?

@mcking65 mcking65 changed the title Occasional timeout when waiting for link to load in regresison tests on github checks Occasional regression test timeout when waiting for link to load Oct 30, 2020
@nschonni
Copy link
Contributor

I guess I was able to merge both of those PRs even with the failures because we haven't yet changed the branch policies (#1548). Is that right @nschonni?

Correct, none of the jobs are marked as required currently except for Travis. I left a note over there for Michael to at least make Travis no longer required. I'm not sure if you wanted to make the new GitHub jobs required, but you can leave a note there if you do.

@spectranaut
Copy link
Contributor Author

@mcking65 a stopgap could be to mark those tests as failing or comment them out, it's only a handful of tests. I don't have a problem doing that as long as it's a priority to get this fixed in the next month or so, which I think we definitely can. When commenting out I can create a list on this issue to "audit" of all external links in examples here and we can decide something reasonable to do.

I like the idea of keeping links to within the APG. I think we should think about the effect of the links in the ARIA-AT repo and also when imported into codepen. Maybe we should just always link back to the spec, maybe we should always link to another dummy page, or maybe we should always link to a hashref? I think turning the link into a popup would be a bit weird for trying to get a feel for the user experience of the widget.

@spectranaut
Copy link
Contributor Author

spectranaut commented Oct 30, 2020

Examples that use external links

  1. Link
    Three examples of role="link", they all link to w3.org

  2. Navigational Menu Button
    List of links to six difference w3c resources

  3. Treeview
    List of links to wikipedia fruit pages

Examples that use interal links

  1. Navigational Menubar
    These links are two a different HTML page. When loading into codepen, and taken to this page, if you click the "go back to menubar example", you are taken to the APG page instead of the example that is represented by the code. However, the browsers "back" button works. Additionally, when you edit the codepen code, the example is rerendered. The resulting experience is ok in Codpen, but it seems like it would be a bit awkward when importing the examples tests into ARIA-AT. To support ARIA-AT, we could remove the "go back to menubar example" and then the tester would just have to use the back button. Otherwise, we would have to sometimes import a second HTML file.

  2. Tree Grid
    In the treegrid example, we replace a "email" link with a url that include a url fragment in order to test the behavior of "enter". We could actually do this for all the tests, and that would eliminate the problem.

@spectranaut
Copy link
Contributor Author

The answer is in the audit! We don't need to change our example policy, we can just change test: we can update the url on the external links to be the same page + a url fragment, like we do in the test in tree grid. It's just three tests so it should be quick and easy :)

@mcking65
Copy link
Contributor

mcking65 commented Nov 8, 2020

@spectranaut love the approach!

@mcking65 mcking65 added enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation labels Nov 8, 2020
@mcking65 mcking65 added this to the 1.2 Release 1 milestone Nov 8, 2020
@mcking65 mcking65 linked a pull request Nov 8, 2020 that will close this issue
mcking65 pushed a commit that referenced this issue Nov 8, 2020
* Fixes #1594 by changing link tests to not follow external links.
* Adds replace external links helper function
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any addition or improvement that doesn't fix a code bug or prose inaccuracy Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants