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

expr: Add tests #7356

Merged
merged 2 commits into from
Mar 5, 2025
Merged

expr: Add tests #7356

merged 2 commits into from
Mar 5, 2025

Conversation

RenjiSann
Copy link
Collaborator

I've rewritten all the GNU testcases of gnu/tests/expr/expr.pl in our testsuite, to more easily track potential regressions.

Sadly, the remaining issues are all caused by our regex dependency (rust-onig) which has a diverging behavior from GNU. This emphasizes the need to find another regex engine, as discussed in #1145

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

@RenjiSann RenjiSann force-pushed the expr-fixes branch 3 times, most recently from cf5aa13 to 5aa8a62 Compare February 27, 2025 10:53
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

@sylvestre sylvestre requested a review from Copilot March 2, 2025 08:24

Choose a reason for hiding this comment

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

PR Overview

This pull request rewrites GNU test cases for expr to better track regressions and highlights issues with our current regex dependency. Key changes include:

  • Renaming the error variant from InvalidContent(String) to InvalidBracketContent.
  • Updating error handling and corresponding tests in syntax_tree.rs.
  • Adjusting error message attributes in expr.rs to align with the new error variant.

Reviewed Changes

File Description
src/uu/expr/src/syntax_tree.rs Renamed error variant usage and updated test assertions.
src/uu/expr/src/expr.rs Modified the error variant declaration and its error message.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/uu/expr/src/expr.rs:50

  • [nitpick] The new static error message may reduce context for debugging. If dynamic error details are useful, consider reintroducing a parameterized variant to capture more specific information.
#[error("Invalid content of \{{\}}"]
Copy link

github-actions bot commented Mar 4, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

@sylvestre sylvestre merged commit d31a19f into uutils:main Mar 5, 2025
65 checks passed
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.

2 participants