Skip to content

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Jul 1, 2023

This PR catches the exception of regex library absence, and gracefully skip the test when built without oniguruma. This resolves #2292, but is better running all the manual tests except for regex ones.

@itchyny itchyny force-pushed the skip-regex-tests-without-oniguruma branch from 422c71d to 8baa1f1 Compare July 1, 2023 16:18
@wader
Copy link
Member

wader commented Jul 1, 2023

Could we use the fact that (nearly?) all regexp related test are in onig.test? if so maybe skip could be done in the build system instead of in the run tests code?

The macOS build failure looks like something temporary:

error: Could not read ffac597fdb6ce8236b5e2da77ab6fbae3d9d9cb7
fatal: revision walk setup failed
error: https://github.com/Homebrew/homebrew-cask did not send all necessary objects

Error: Fetching /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask failed!

@itchyny
Copy link
Contributor Author

itchyny commented Jul 3, 2023

But we need this code to make man.test pass without regexp support after all.

@wader
Copy link
Member

wader commented Jul 3, 2023

But we need this code to make man.test pass without regexp support after all.

Aha i see 👍

Maybe if we end up with more conditional tests in the future the .test format could be extended with some kind of feature gating?

@itchyny
Copy link
Contributor Author

itchyny commented Jul 3, 2023

Hmm, let me reconsider merging and explore about improving the .test format. How about adding %%REGEX sign to indicate the test case requires the regex support, including the regex: true flag in the manual source file, and skip them unless HAVE_LIBONIG?

@wader
Copy link
Member

wader commented Jul 3, 2023

Maybe use the config vars and have %%HAVE_LIBOING or just %%LIBOING would make it similar to what we do in c code. But i like %%REGEX also, more feature name than implementation detail... but i guess jq is not changing regex implementation anytime soon :)

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

Successfully merging this pull request may close these issues.

2 participants