Skip to content

[Cache] Fix two initializations of Redis Sentinel #51775

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

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

digilist
Copy link
Contributor

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets n/a
License MIT

After #51687 and #51683 have been merged, there are now two initializations of Redis Sentinel, which also leads to the integration test failure mentioned in this comment. This PR fixes this.

@nicolas-grekas
Copy link
Member

Thank you @digilist.

@stof
Copy link
Member

stof commented Sep 28, 2023

As you said in #51687 (comment) that you copied the bad usage of SkippedTestSuiteError from the tests of the Cache component, maybe you could update the test too (assuming older branches don't also need the fix, in which case a separate PR is better)

@nicolas-grekas nicolas-grekas merged commit 1b0aed0 into symfony:6.4 Sep 28, 2023
@digilist
Copy link
Contributor Author

Yep. Since this PR is already merged I opened a new PR for it.

nicolas-grekas added a commit that referenced this pull request Sep 28, 2023
…pped() call (digilist)

This PR was merged into the 6.4 branch.

Discussion
----------

Replace usages of SkippedTestSuiteError with markTestSkipped() call

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | n/a
| License       | MIT

Just a simple clean up to replace the usages of `SkippedTestSuiteError` with a call to `markTestSkipped()`.

This is a follow up for #51775.

Commits
-------

41eb669 Replace usages of SkippedTestSuiteError with markTestSkipped() call
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants