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

feat: add a fuzz test for kclvm-parser.parse_expr. #32

Merged
merged 2 commits into from
May 25, 2022

Conversation

zong-zhe
Copy link
Contributor

Note:

  1. "PanicInfo.to_json_string" has been modified.
  2. Some problems detected by the fuzz test will be solved soon.

What:

  1. Add a fuzz test for kclvm-parser.parse_expr.
  2. "PanicInfo.to_json_string" has been modified.
  3. Remove some unusd imports.
  4. In order to adapt to the new "PanicInfo.to_json_string",
    some code have been changed.

How:

  1. Add "fuzz/fuzz_parser" in kclvm-parser for "parse_expr".
  2. Remove some unusd imports.
  3. "PanicInfo.to_json_string" has been modified by "serde_json::to_string".
  4. Change all “$kcl_PanicInfo$” to “kcl_PanicInfo”.

Why:

  1. Add a fuzz test in order to detect more bugs.
  2. "serde" provides a better interface for serialization/deserialization of “PanicInfo”.
  3. Serialization/Deserialization of “PanicInfo" is helpful for catching panic errors in fuzz.
  • With pull requests:
    • Open your pull request against main
    • Your pull request should have no more than two commits, if not you should squash them.
    • It should pass all tests in the available continuous integration systems such as GitHub Actions.
    • You should add/modify tests to cover your proposed code changes.
    • If your pull request contains a new feature, please document it on the README.

@zong-zhe zong-zhe requested review from chai2010 and Peefy May 24, 2022 10:46
Note:
1. "PanicInfo.to_json_string" has been modified.
2. Some problems detected by the fuzz test will be solved soon.

What:
1. Add a fuzz test for kclvm-parser.parse_expr.
2. "PanicInfo.to_json_string" has been modified.
3. Remove some unusd imports.
4. In order to adapt to the new "PanicInfo.to_json_string",
   some code have been changed.

How:
1. Add "fuzz/fuzz_parser" in kclvm/tests.
2. Init kclvm/tests to a cargo project.
3. Add fuzz-parser in Makefile.
4. Remove some unusd imports.
5. "PanicInfo.to_json_string" has been modified by "serde_json::to_string".
6. Change all “$__kcl_PanicInfo__$” to “__kcl_PanicInfo__”.

Why:
1. Add a fuzz test in order to detect more bugs.
2. "serde" provides a better interface for serialization/deserialization of “PanicInfo”.
3. Serialization/Deserialization of “PanicInfo" is helpful for catching panic errors in fuzz.
4. "cargo fuzz" can only be used under a cargo project.
@zong-zhe zong-zhe force-pushed the dev/zong-zhe/add_kclvm_fuzzer branch from 688b9ce to 4f766e3 Compare May 25, 2022 03:19
Peefy
Peefy previously approved these changes May 25, 2022
Copy link
Contributor

@Peefy Peefy 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
Contributor

@chai2010 chai2010 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
Contributor

@chai2010 chai2010 left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@Peefy Peefy left a comment

Choose a reason for hiding this comment

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

LGTM

@Peefy Peefy merged commit e46caab into main May 25, 2022
@zong-zhe zong-zhe deleted the dev/zong-zhe/add_kclvm_fuzzer branch June 2, 2022 02:11
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.

3 participants