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 rz-test '-y' option to accept all #4772

Merged
merged 2 commits into from
Dec 22, 2024
Merged

add rz-test '-y' option to accept all #4772

merged 2 commits into from
Dec 22, 2024

Conversation

imbillow
Copy link
Contributor

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

...

Test plan

...

Closing issues

...

Copy link
Member

@kazarmy kazarmy left a comment

Choose a reason for hiding this comment

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

Why do you want this -y option?

@imbillow
Copy link
Contributor Author

Why do you want this -y option?

Because sometimes changing some of rzil's code creates a large number of changes

@kazarmy
Copy link
Member

kazarmy commented Dec 17, 2024

Do you have an example of that? ... sorry not that keen on adding code that has to be manually tested and is not strictly necessary

@imbillow
Copy link
Contributor Author

imbillow commented Dec 17, 2024

Do you have an example of that? ... sorry not that keen on adding code that has to be manually tested and is not strictly necessary

042f917

For example, in this commit, I used this feature to automatically ACCEPT over 200 asm tests at a time.

Well, if I'm the only one who uses this feature, then it's okay to turn off the PR.

Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

It's actually easy to create a test for this @kazarmy - there is test/unit/test_rz_test.c

binrz/rz-test/rz-test.c Show resolved Hide resolved
@kazarmy
Copy link
Member

kazarmy commented Dec 18, 2024

It's actually easy to create a test for this ...

Well actually it's not because whether it can be tested automatically, it's because I don't think blind fixing should be encouraged, but I suppose it's fine as long as it's used responsibly

@XVilka XVilka merged commit 1330aa7 into dev Dec 22, 2024
47 of 48 checks passed
well-mannered-goat pushed a commit to well-mannered-goat/rizin that referenced this pull request Dec 24, 2024
* Add rz-test '-y' option to accept all
* Add '-y' into the help text
well-mannered-goat added a commit to well-mannered-goat/rizin that referenced this pull request Dec 26, 2024
chore:updated rz_core_asm_plugins_print to have flag arguments

add optional flags support to rz_core_asm_plugins_print

add feature argument

add features argument

rz-test: add `-y` option to accept all (rizinorg#4772)

* Add rz-test '-y' option to accept all
* Add '-y' into the help text

add clang format

correct Laj output to have features other than ad

add tests for filtering La

add tests for filtering Laq

add tests for filtering Laj

clang format

rz-test: add `-y` option to accept all (rizinorg#4772)

* Add rz-test '-y' option to accept all
* Add '-y' into the help text

dbtj: Add space between flags in `desc` property (rizinorg#4790)

subprojects: update rz-libdemangle (rizinorg#4791)

Remove trailing space in dbg_trace test (rizinorg#4792)

merge the filtering tests

add clang format

Remove trailing space in dbg_trace test (rizinorg#4792)

rzil: add VM event memory (read|write) index (rizinorg#4789)

This is because the memory of the RzIL VM has an index, but the index is ignored when an event is generated.

revert changes
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.

3 participants