Problem/Motivation

When testing with 7.4, there are several tests within ProjectBrowserUITest that fail often.

Steps to reproduce

Proposed resolution

Will require some trial and error, but at first glance it seems to be a timing issue. In some places, there are waitForElementVisible calls after changing filters, but it's waiting on an element (.project) that exists but is removed when a filter change is made. If the tests are running faster or the UI is slightly slower, the wait for .project might not work as hoped.

First thing to try is some waits that are more specific to what is being waited on for assertions that aren't preceded by something that provides a sufficient wait. This basically means using waitForText after UI changes that don't have other waits prior to checking for the text.

Remaining tasks

  • ✅ File an issue about this project
  • ☐ Manual Testing
  • ☐ Code Review
  • ☐ Accessibility Review
  • ☐ Automated tests needed/written?
CommentFileSizeAuthor
#10 another-30x.patch18.79 KBbnjmnm
#9 testing-forty-times.patch18.54 KBbnjmnm

Issue fork project_browser-3300447

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bnjmnm created an issue. See original summary.

bnjmnm opened merge request !213

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Issue summary: View changes
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Active » Needs review
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Assigned: bnjmnm » Unassigned
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

These changes make perfect sense to me.

Is there any sense in many having a no-commit MR that runs the test 500x, just to be sure we're not introducing (or suffering from) random failures? I leave that decision to @bnjmnm or another person who knows more than I do; otherwise I'd feel good RTBCing this.

phenaproxima opened merge request !215

fjgarlin’s picture

fjgarlin commented
Status: Needs review » Reviewed & tested by the community

Curious to see the result of MR 215. But regarding MR 213, the changes make sense, so marking as RTBC. The readability is not ideal, but as Tim mentioned (via slack), we'll probably copy/paste from existing tests, and since these tests are functional on a heavily JS page, I'm also not surprised, so I think it's good.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
StatusFileSize
new testing-forty-times.patch18.54 KB
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
StatusFileSize
new another-30x.patch18.79 KB
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented

Based on the rounds above, it looks like PHP 7.4 is passing reliably and 7.3 is a little shaky. It's completely possible that 7.4 will still have the occasional random, but at this point I don't believe it's more prone to random fails than 7.3 and the test runs suggest the opposite.

tim.plunkett made their first commit to this issue’s fork.

  • tim.plunkett committed 7550e27 on 1.0.x authored by bnjmnm 
    Issue #3300447 by bnjmnm, phenaproxima: Fragile assertions in...
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Reviewed & tested by the community » Fixed

Given the follow-up of #3300735: Address ProjectBrowserUITest workaround (which I just bumped to major), I think this is good enough now. Thanks @bnjmnm for the incredible effort here, and for @phenaproxima for pitching in

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.