Problem/Motivation

We have a dedicated runner for performance tests, tagged with 'performance-test-runner', if we add that tag to performance tests, they should hopefully match up.

Steps to reproduce

Proposed resolution

Tag the performance test job with a tag, this tag matches the performance test runner tag.

Additionally fixes webdriver error handling.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3415296

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

catch created an issue. See original summary.

catch opened merge request !6211

sudhakartag1 made their first commit to this issue’s fork.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review

Green test run with the performance job schedule condition removed: https://git.drupalcode.org/project/drupal/-/jobs/687936.

catch’s picture

catch
he/him
Primary language English
commented
smustgrave’s picture

smustgrave commented

Could the IS be updated for why change to core/tests/Drupal/FunctionalJavascriptTests/WebDriverCurlService.php was needed? Assuming for title this was going to be a pipeline only fix.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes

It might need splitting to its own issue - it fixes things so that if chromedriver produces an error, we get the error message instead of a cryptic type error from PHP. Update the issue summary for now.

smustgrave’s picture

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

Thanks for updating, will lean on your expertise on this one too. Maybe a follow up should be made though for testing that chromedriver errors are properly captured, or is that overkill?

catch’s picture

catch
he/him
Primary language English
commented

Not a huge fan of testing the test framework - in this case the phpstan baseline needed to be updated, so we wouldn't be able to regress in this specific way again.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Going to open a followup to see if we can remove sudo from all tests, I don't think we need it on GitLab, and it makes the CI config more complicated.

I think it's fine to fix the WebDriverCurlService bug here, PHPStan was telling us there was an issue already so no need for a specific test.

Backported to 10.2.x to keep things in sync.

Committed and pushed b35de78f74 to 11.x and e48f905814 to 10.2.x. Thanks!

  • longwave committed e48f9058 on 10.2.x 
    Issue #3415296 by catch, sudhakartag1, smustgrave: Use a dedicated...

  • longwave committed b35de78f on 11.x 
    Issue #3415296 by catch, sudhakartag1, smustgrave: Use a dedicated...

longwave closed merge request !6211

Status: Fixed » Closed (fixed)

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