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(#341): Allow comma and brackets in matcher expressions #977

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

tschlat
Copy link
Collaborator

@tschlat tschlat commented Sep 8, 2023

I am pleased to provide the following fix for Issue 341.
As stated in the issue the problem here are the comma and bracket chars in the matcher expression. They cannot be parsed into the correct matcher parameters using the simple approach, splitting by ','.

Note that for the specific case, the matcher text needs to be put in '', as it contains '(' and ''' characters which would otherwise interfere with the parameter extraction algorithm.

That said, the following expression:

allOf(startsWith('INSERT INTO todo_entries (id, title, description, done)'))

Would perfectly match this string:

INSERT INTO todo_entries (id, title, description, done) values (1, 'Invite for meeting', 'Invite the group for a lunch meeting', 'false')

@tschlat tschlat linked an issue Sep 8, 2023 that may be closed by this pull request
@tschlat tschlat self-assigned this Sep 8, 2023
@tschlat tschlat requested review from bbortt and christophd September 8, 2023 12:49
@tschlat tschlat force-pushed the issue/341/allow_comma_in_matcher_expressions branch 4 times, most recently from 1822612 to d22b822 Compare September 8, 2023 12:57
Copy link
Collaborator

@bbortt bbortt left a comment

Choose a reason for hiding this comment

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

I like it! the provided test case does perfectly display its functionality.

formatting could be a little more consistent, tho. but who am I to judge 😉

@tschlat
Copy link
Collaborator Author

tschlat commented Sep 8, 2023

I like it! the provided test case does perfectly display its functionality.

formatting could be a little more consistent, tho. but who am I to judge 😉

Thanks for the flowers Mr. Eagle Eye Borter!

I still want to come up with a solution, that also allows the ' in the param string.
Working on it. Stay tuned.

@tschlat tschlat force-pushed the issue/341/allow_comma_in_matcher_expressions branch 3 times, most recently from 2836841 to 300295f Compare September 8, 2023 22:13
@tschlat
Copy link
Collaborator Author

tschlat commented Sep 8, 2023

I guess thats it.

@tschlat tschlat force-pushed the issue/341/allow_comma_in_matcher_expressions branch 4 times, most recently from 0734349 to bd3d874 Compare September 9, 2023 14:55
@tschlat
Copy link
Collaborator Author

tschlat commented Sep 9, 2023

Added some more tests. I think this is now ready for final review.

@tschlat tschlat force-pushed the issue/341/allow_comma_in_matcher_expressions branch from bd3d874 to 2b8fccc Compare September 11, 2023 09:00
Copy link
Collaborator

@bbortt bbortt left a comment

Choose a reason for hiding this comment

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

just saw some formatting issues. all good otherwise.

@tschlat tschlat force-pushed the issue/341/allow_comma_in_matcher_expressions branch from 2b8fccc to dcdab5e Compare September 12, 2023 04:49
@bbortt bbortt merged commit f210123 into main Sep 12, 2023
1 check passed
@bbortt bbortt deleted the issue/341/allow_comma_in_matcher_expressions branch September 12, 2023 08:05
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.

Validating SQL with Hamcrest matchers throws OutOfBoundException
2 participants