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: Complex test name normalization #139

Merged
merged 7 commits into from
Dec 23, 2023
Merged

fix: Complex test name normalization #139

merged 7 commits into from
Dec 23, 2023

Conversation

flohw
Copy link
Contributor

@flohw flohw commented Dec 22, 2023

Hello!

As a Christmas gift I come here to preset the work to hopefully close #67 properly 🎅 🥳

I added the cases suggested by @0x346e3730 based on the existing test cases of @raneomik.

Let me know if the new algorithm is ok for you.
The regexes are a bit stricter as there is with data set into, that's why I return early on line 117. It make the regexes a bit easier to read I think. That's not perfect to my eyes (depending on a static string is not algorithmically valid to me) but it may be enough. WDYT?
I will also create named group to make the code clearer.

I didn't set cases stated by er1z in the issue as he didn't explain much about its problem.

If you look at it during this long weekend, please don't merge. I wanted to sign my commits but I have some issue with my passphrase memory 😅

Thanks and have a nice week-end!

[EDIT]: Failing test seems unrelated.

@kbond
Copy link
Member

kbond commented Dec 22, 2023

Awesome, thanks for working on this @flohw! Let me know when it's ready and I'll merge!

@kbond kbond marked this pull request as draft December 22, 2023 18:53
@flohw
Copy link
Contributor Author

flohw commented Dec 23, 2023

Hi @kbond,

Chocolate activates some neurons! I added the regex group names to be more comprehensible. Also I updated the preg_replace to replace multiple non-word characters by a single hyphen to shorten the filename a bit.

For example for the dataset single quote for array index access with {{[\'id\']|filter(\'system\')}}st as a dataset name, the {{[ was transformed to --- in my previous version, it is a single - now.

Let me know if I need to make changes you have in mind.

Have a nice week-end!

@flohw flohw marked this pull request as ready for review December 23, 2023 09:07
@kbond kbond merged commit 7144c6a into zenstruck:1.x Dec 23, 2023
20 checks passed
@kbond
Copy link
Member

kbond commented Dec 23, 2023

Thanks @flohw!

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.

Undefined array key 1 when the test failed
2 participants