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

Fix regex support in --only, --skipfile and --skiptest #10741

Merged
merged 4 commits into from May 25, 2022

Conversation

valentinogeron
Copy link
Contributor

@valentinogeron valentinogeron commented May 17, 2022

The regex support was added in:

These commits break backword compatiblity with older versions.

This fix keeps the test suite infra compatible with old versions by
default. However, if you want regex, the string must start with /

@oranagra
Copy link
Member

I'm not sure the approach of trying both is right.
It means we may skip something we didn't mean to skip.
I suppose the right thing to do is have an argument or prefix that indicate the type of string matching.

I'm AFK, but I suppose all of these got broken in 7.0, so maybe we should just "revert" it, and introduce it again properly.
I.e. Make the default as it was before, and the regex thing optional.

@oranagra oranagra added this to Backlog in 7.0 via automation May 18, 2022
@oranagra oranagra moved this from Backlog to In Review in 7.0 May 18, 2022
@oranagra
Copy link
Member

I discussed this with Yossi, we decided to be compatible with old versions by default and if you want regex the string must start with /
@valentinogeron can you change your PR?

@valentinogeron
Copy link
Contributor Author

I discussed this with Yossi, we decided to be compatible with old versions by default and if you want regex the string must start with / @valentinogeron can you change your PR?

@oranagra - yes

The regex support was added in:
 * redis#9352
 * redis#9555
 * redis#10212

These commits break backword compatiblity with older versions.

This fix keeps the testsuite infra compatible with old versions by
default. If you want regex the string must start with `/`
tests/support/test.tcl Outdated Show resolved Hide resolved
tests/test_helper.tcl Outdated Show resolved Hide resolved
trim the / from the expression
fix documentation
tests/instances.tcl Outdated Show resolved Hide resolved
tests/support/test.tcl Outdated Show resolved Hide resolved
search_pattern_list gets an optional argument for asking sub-string
match only.
tests/support/test.tcl Show resolved Hide resolved
tests/test_helper.tcl Outdated Show resolved Hide resolved
@oranagra oranagra merged commit 9eb97b5 into redis:unstable May 25, 2022
@oranagra oranagra moved this from In Review to Unreleased in 7.0 May 25, 2022
@oranagra oranagra moved this from Unreleased to Done in 7.0 Jun 13, 2022
Mixficsol pushed a commit to Mixficsol/redis that referenced this pull request Apr 12, 2023
The regex support was added in:
 * redis#9352
 * redis#9555
 * redis#10212

These commits break backword compatiblity with older versions.

This fix keeps the test suite infra compatible with old versions by
default. However, if you want regex, the string must start with `/`
enjoy-binbin pushed a commit to enjoy-binbin/redis that referenced this pull request Jul 31, 2023
The regex support was added in:
 * redis#9352
 * redis#9555
 * redis#10212

These commits break backword compatiblity with older versions.

This fix keeps the test suite infra compatible with old versions by
default. However, if you want regex, the string must start with `/`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
7.0
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants