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

test: support regular expression matching on the response #16472

Merged
merged 2 commits into from
Aug 25, 2023

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Aug 25, 2023

Currently we only support exact string match on the response (e.g. curl response, and process output). But in some cases, the response may not be a fixed string. One example is discussed in #16454, there may be random spaces in the grpc-gateway v2 response.

So we need to support match the response based on a regular expression.

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@ahrtr ahrtr force-pushed the expr_expect_20230825 branch 2 times, most recently from 14ef651 to 8c60e12 Compare August 25, 2023 10:30
@ahrtr
Copy link
Member Author

ahrtr commented Aug 25, 2023

Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks @ahrtr.

Noting expect.ExpectResponse feels a bit verbose/duplicated naming but I can't think of a better way of putting it in relation to the rest of the package.

@ahrtr ahrtr force-pushed the expr_expect_20230825 branch from 8c60e12 to 7d95c68 Compare August 25, 2023 14:01
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@ahrtr
Copy link
Member Author

ahrtr commented Aug 25, 2023

ping @wenjiaswe @serathius @spzala PTAL when you get free cycle. This should be a simple and straightforward change.

I expect this PR to be merged asap, otherwise it will definitely conflict with other PRs.

@wenjiaswe
Copy link
Contributor

lgtm

@ahrtr
Copy link
Member Author

ahrtr commented Aug 25, 2023

thx @wenjiaswe for the quick review. Also thx @jmhbnz and @fuweid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants