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

[CMSP-459] Updates behat tests and behaviors. #430

Merged
merged 5 commits into from
Jun 23, 2023
Merged

Conversation

rwagner00
Copy link
Contributor

@rwagner00 rwagner00 commented Jun 12, 2023

Ports behat fixes from https://github.com/pantheon-systems/wp-redis/pull/429/files into its own PR.

This PR also removes the terminus redis:enable step in bin/behat-prepare.sh. There's no reason to ever disable redis on the fixture running Behat tests, and the enable step spawns many duplicate "redis enable" requests, most of which time-out, causing the tests to fail.

Tests can be seen as passing (with #437) here: https://app.circleci.com/pipelines/github/pantheon-systems/wp-redis/1629/workflows/fd714abf-075f-45f8-9102-3ddc730f731e/jobs/4980

@rwagner00 rwagner00 requested review from a team as code owners June 12, 2023 21:08
@pwtyler
Copy link
Member

pwtyler commented Jun 13, 2023

Are some of these tests specific to #426 and therefore now failing on this branch?

@jazzsequence
Copy link
Contributor

it's possible the base branch of this PR should be update-object-cache

@pwtyler
Copy link
Member

pwtyler commented Jun 15, 2023

There were both test regressions (tests needed to be fixed) and legitimate errors (code needed to be fixed) @rwagner00 addressed in getting #426 green. Anything that fixes default's failing tests should be split out to this PR, anything else should remain and be fixed up in 426 after this is merged. I'm not sure if the legitimate errors are errors introduced in 426, or in one of our recent merges to default though.

we end up making a dozen differnet enable redis requests and inevitably a lot of those fail because there are so many happening simultaneously (assumption). It's okay to just enable it and leave it on, rather than turning it on and off.
@jazzsequence jazzsequence changed the title Updates behat tests and behaviors. [CMSP-459] Updates behat tests and behaviors. Jun 22, 2023
Copy link
Member

@pwtyler pwtyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, as I understand it takes merging both this and #437 to 🟢 behat on default branch

@jazzsequence jazzsequence merged commit 7248457 into default Jun 23, 2023
@jazzsequence jazzsequence deleted the behat-fixes branch June 23, 2023 14:58
jazzsequence added a commit that referenced this pull request Jun 26, 2023
* Updates behat tests and behaviors.

* Updates behat tests and behaviors.

* port diff from #426

* tabs to spaces

* don't enable redis as part of the prepare step
we end up making a dozen differnet enable redis requests and inevitably a lot of those fail because there are so many happening simultaneously (assumption). It's okay to just enable it and leave it on, rather than turning it on and off.

---------

Co-authored-by: Ryan Wagner <[email protected]>
Co-authored-by: Chris Reynolds <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants