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 test project for the Bazel fuzzing rules. #4936

Merged
merged 7 commits into from
Jan 22, 2021

Conversation

stefanbucur
Copy link
Contributor

This PR is in reference to bazel-contrib/rules_fuzzing#105. This is a disabled OSS-Fuzz project that builds all the fuzz target examples in bazelbuild/rules_fuzzing and generates the OSS-Fuzz packaging for them.

Question to reviewers: Is there a way this project can be used to test new changes on a branch in bazelbuild/rules_fuzzing? Right now the Dockerfile hard-codes the master branch when it clones the repo. Can this be parameterized somehow? Has anyone done this before?

@inferno-chromium
Copy link
Collaborator

This PR is in reference to bazelbuild/rules_fuzzing#105. This is a disabled OSS-Fuzz project that builds all the fuzz target examples in bazelbuild/rules_fuzzing and generates the OSS-Fuzz packaging for them.

Question to reviewers: Is there a way this project can be used to test new changes on a branch in bazelbuild/rules_fuzzing? Right now the Dockerfile hard-codes the master branch when it clones the repo. Can this be parameterized somehow? Has anyone done this before?

Usually people just specify it in dockerfile clone command, we don't support different project version with params. can you please tell the usecase.

Copy link
Collaborator

@inferno-chromium inferno-chromium left a comment

Choose a reason for hiding this comment

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

LGTM with minor changes.

projects/bazel-rules-fuzzing-test/Dockerfile Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/Dockerfile Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/Dockerfile Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/build.sh Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/build.sh Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/project.yaml Outdated Show resolved Hide resolved
@stefanbucur
Copy link
Contributor Author

This PR is in reference to bazelbuild/rules_fuzzing#105. This is a disabled OSS-Fuzz project that builds all the fuzz target examples in bazelbuild/rules_fuzzing and generates the OSS-Fuzz packaging for them.
Question to reviewers: Is there a way this project can be used to test new changes on a branch in bazelbuild/rules_fuzzing? Right now the Dockerfile hard-codes the master branch when it clones the repo. Can this be parameterized somehow? Has anyone done this before?

Usually people just specify it in dockerfile clone command, we don't support different project version with params. can you please tell the usecase.

We discussed with Oliver that this test project could be used as a regression test in a bazel_rules Github action. But for this to work on PRs, the branch of the PR would actually need to be checked out and tested here. For example, if we make changes to the OSS-Fuzz rules, right now we'd not be able to find any issues before we actually merge the PR.

Copy link
Contributor Author

@stefanbucur stefanbucur left a comment

Choose a reason for hiding this comment

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

PTAL.

projects/bazel-rules-fuzzing-test/Dockerfile Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/Dockerfile Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/Dockerfile Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/build.sh Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/build.sh Outdated Show resolved Hide resolved
projects/bazel-rules-fuzzing-test/project.yaml Outdated Show resolved Hide resolved
#
################################################################################

declare -r QUERY='
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is test project, it is good to add comments on each step in this file. Can you do this in next PR. Also, need to remember to add docs on subsection of https://google.github.io/oss-fuzz/getting-started/new-project-guide/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this is test project, it is good to add comments on each step in this file. Can you do this in next PR.

I've just seen this. Let me do this in #5039.

Also, need to remember to add docs on subsection of https://google.github.io/oss-fuzz/getting-started/new-project-guide/

Good point, let me do this in a subsequent PR. I'm planning to overhaul documentation in the rules_fuzzing library, too.

@inferno-chromium inferno-chromium merged commit 5f6c8ad into google:master Jan 22, 2021
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