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

Add tests for builtin strings::indexof method #311

Merged

Conversation

Sumynwa
Copy link
Contributor

@Sumynwa Sumynwa commented Sep 10, 2024

Partly fixes #12

@Sumynwa Sumynwa force-pushed the sumsharma/builtin_strings_indexof_tests branch from e795e86 to 99bf7d3 Compare September 10, 2024 14:51
@anakrish
Copy link
Collaborator

@Sumynwa Thanks for the contribution.

There is a failure unrelated to your change, when building the Java binding. It would be great if you can fix that too:

error: match can be simplified with `.unwrap_or_default()`
  --> src/utils.rs:81:5
   |
81 | /     match get_extra_arg_impl(expr, module, functions) {
82 | |         Ok(a) => a,
83 | |         _ => None,
84 | |     }
   | |_____^ help: replace it with: `get_extra_arg_impl(expr, module, functions).unwrap_or_default()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or_default
   = note: `-D clippy::manual-unwrap-or-default` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::manual_unwrap_or_default)]`

error: could not compile `regorus` (lib) due to 1 previous error

@Sumynwa
Copy link
Contributor Author

Sumynwa commented Sep 10, 2024

@anakrish Yes, I will fix the clippy lint error.

@anakrish
Copy link
Collaborator

@Sumynwa I have fixed the clippy lint error as part of another PR. Can you rebase to latest main and push?

@Sumynwa Sumynwa force-pushed the sumsharma/builtin_strings_indexof_tests branch from 99bf7d3 to ddc0834 Compare September 11, 2024 06:39
Copy link
Collaborator

@anakrish anakrish left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@anakrish anakrish left a comment

Choose a reason for hiding this comment

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

LGTM

@anakrish anakrish merged commit ecd341b into microsoft:main Sep 11, 2024
16 checks passed
@Sumynwa Sumynwa deleted the sumsharma/builtin_strings_indexof_tests branch September 11, 2024 16:21
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.

[Testing] Add test cases for strings builtins
2 participants