Skip to content

Add module-level documentation to lib.rs #545

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

Merged

Conversation

markv44
Copy link
Contributor

@markv44 markv44 commented Apr 10, 2025

Description

This PR addresses issue #543 by adding comprehensive module-level documentation to the main library file cargo-test-fuzz/src/lib.rs. The documentation provides an overview of the crate's purpose, its primary features, and how it integrates with the larger test-fuzz project.

Changes

  • Added a module-level documentation block using //! doc comments at the beginning of the file
  • Included information about the crate's core functionality and features
  • Explained how the crate relates to the cargo test-fuzz subcommand

Related Issue

Closes #543

Checklist

  • Documentation added
  • Code follows project style and conventions
  • Changes don't break existing functionality

Added comprehensive module-level documentation to cargo-test-fuzz/src/lib.rs
explaining the crate's purpose, functionality, and how it integrates with
the test-fuzz project.
@markv44 markv44 requested a review from smoelius as a code owner April 10, 2025 16:06
@CLAassistant
Copy link

CLAassistant commented Apr 10, 2025

CLA assistant check
All committers have signed the CLA.

@markv44
Copy link
Contributor Author

markv44 commented Apr 10, 2025

@smoelius please check if the documentation I added is to your liking, if you think its good then please merge it. Thank you.

@markv44
Copy link
Contributor Author

markv44 commented Apr 11, 2025

hey @smoelius could you please approve the ci tests?

@markv44
Copy link
Contributor Author

markv44 commented Apr 11, 2025

@smoelius I think this can be merged now.

Comment on lines 6 to 16
//! The primary features of this crate include:
//!
//! - Building and running fuzz tests using `afl.rs`
//! - Managing and generating fuzzing corpora
//! - Displaying and replaying crashes and hangs
//! - Consolidating and resetting test output
//!
//! This crate provides the backend for the `cargo test-fuzz` subcommand, which allows users to:
//! - Run fuzzing operations against test targets annotated with the `test_fuzz` macro
//! - Manage fuzzing artifacts (corpus, crashes, hangs)
//! - Configure fuzzing parameters (timeout, CPU usage, etc.)
Copy link
Collaborator

@smoelius smoelius Apr 12, 2025

Choose a reason for hiding this comment

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

I think the number of users that use cargo-test-fuzz as a library is small, and maybe even zero.

Such users are likely to already know what cargo-test-fuzz is. So I doubt this text would be helpful to them.

To be honest, I think this text can be eliminated.

Ideally, it would be replaced with a reference to run since that is the library's main export:

pub fn run(opts: TestFuzz) -> Result<()> {

But I would understand if you don't want to go to that trouble.

EDIT: Just as a followup, #543 mentioned these things:

Object enum, TestFuzz struct, run function

I think there would be value in addressing those in the documentation. If someone were to use this crate as a library, those would be the things that they use.


Please forgive me for asking, but was this generated with an LLM?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smoelius About the initial documentation - I had GitHub Copilot enabled and after writing the first couple of lines, it suggested the rest of the content. Since it was a simple documentation fix and the suggestions looked reasonable, I went with it after reviewing.

I've changed the documentation so that it is focused on documenting the primary exports as you mentioned. I wrote this version myself without relying on Copilot's suggestions, hope this won't cause any problems.

Copy link
Collaborator

@smoelius smoelius Apr 12, 2025

Choose a reason for hiding this comment

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

@smoelius About the initial documentation - I had GitHub Copilot enabled and after writing the first couple of lines, it suggested the rest of the content. Since it was a simple documentation fix and the suggestions looked reasonable, I went with it after reviewing.

Wow. I was not aware Copliot did that. Thank you for explaining.

I've changed the documentation so that it is focused on documenting the primary exports as you mentioned. I wrote this version myself without relying on Copilot's suggestions, hope this won't cause any problems.

What you have now is a definite improvement to the code. Thank you!

While you're here, would you mind adding #[doc(hidden)] to run_without_exit_code? Your changes caused me to realize that function should not be public. I would prefer to not remove the pub now since that would be a breaking change, but we could at least hide the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smoelius I've added the #[doc(hidden)] to run_without_exit_code as you requested, please check if I have to make any more changes.

Updated the documentation for the cargo-test-fuzz crate to clarify its purpose and functionality.
@smoelius
Copy link
Collaborator

The CI failure is not your PR's fault. I'll fix it.

Introduced a hidden doc comment for the run_without_exit_code function in lib.rs to improve code clarity without exposing it in the public API.
@markv44
Copy link
Contributor Author

markv44 commented Apr 12, 2025

@smoelius could you approve the ci test? or are you working to fix the test rn?

@smoelius
Copy link
Collaborator

@smoelius could you approve the ci test? or are you working to fix the test rn?

Sorry for the delay. The CI failure should be fixed now.

@markv44
Copy link
Contributor Author

markv44 commented Apr 12, 2025

@smoelius the ci tests have passes, can this be merged now?

@smoelius smoelius added this pull request to the merge queue Apr 12, 2025
@smoelius
Copy link
Collaborator

Thanks, @markv44!

Merged via the queue into trailofbits:master with commit 5c2d4fe Apr 12, 2025
20 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.

Missing module-level documentation in lib.rs
3 participants