Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions git-branchless-lib/src/core/node_descriptors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,7 @@ impl NodeDescriptor for SignatureStatusDescriptor {
#[cfg(test)]
mod tests {
use std::ops::{Add, Sub};
use std::str::FromStr;
use std::time::Duration;

use super::*;
Expand Down Expand Up @@ -650,4 +651,33 @@ Differential Revision: phabricator.com/D123";

Ok(())
}

#[test]
fn test_signature_status_descriptor() -> eyre::Result<()> {
// Create a mock where is_enabled is false
let mut descriptor = SignatureStatusDescriptor {
is_enabled: false,
repo_path: String::from("/tmp"),
};

// Create a mock object - doesn't matter what it contains since is_enabled is false
let object = NodeObject::GarbageCollected {
oid: NonZeroOid::from_str("1234567890123456789012345678901234567890").unwrap(),
};

// With is_enabled = false, it should return None
let glyphs = Glyphs::text();
let result = descriptor.describe_node(&glyphs, &object)?;
assert!(result.is_none());

// Now test with is_enabled = true
descriptor.is_enabled = true;

// We can't easily test the actual git command execution in a unit test,
// but we can verify that the method doesn't panic and returns a result
let result = descriptor.describe_node(&glyphs, &object);
assert!(result.is_ok());

Ok(())
}
}
92 changes: 92 additions & 0 deletions git-branchless-smartlog/tests/test_smartlog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -812,3 +812,95 @@ fn test_exact() -> eyre::Result<()> {

Ok(())
}

#[test]
fn test_show_signature_flag() -> eyre::Result<()> {
let git = make_git()?;

git.init_repo()?;

// Just create a regular commit without trying to sign it
git.commit_file("signed_file", 1)?;
git.commit_file("unsigned_file", 2)?;
Comment on lines +822 to +824
Copy link
Collaborator

Choose a reason for hiding this comment

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

[question] Are these filenames/comments correct? I don't know much about signing commits, but it seems like something is missing to actually do the signing.


// Run smartlog without --show-signature flag
{
let stdout = git.smartlog()?;
// Without the flag, the signature indicators shouldn't be visible
// However, we can't reliably check for absence of brackets since they might be used elsewhere
// So we'll just ensure the command runs successfully
assert!(!stdout.is_empty(), "Smartlog output should not be empty");
}

// Run smartlog with --show-signature flag
{
let (stdout, _) = git.branchless("smartlog", &["--show-signature"])?;
// With the flag, the command should run successfully
assert!(!stdout.is_empty(), "Smartlog output should not be empty");
// The signature status indicators should be present in some form,
// but we don't validate exactly which status since that depends on the environment
}
Comment on lines +826 to +842
Copy link
Collaborator

Choose a reason for hiding this comment

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

[please change] Many tests in this project use insta snapshots to capture the output of commands, and I think that it would be best for these tests to follow suit. Snapshots can be tedious to keep updated, but they tend to be much more pleasant to debug. You can see the test above (lines 800-810) for an example.

The first assertion here, that the smartlog works without the flag, may not be needed, since the rest of this file already tests that. It can be handy to have when running a single test though; in that case, it should also use a snapshot.


Ok(())
}

#[test]
fn test_show_signature_snapshot() -> eyre::Result<()> {
let git = make_git()?;

git.init_repo()?;

// Create regular commits without trying to sign them
git.commit_file("signed_file", 1)?;

// Create a branch and make another commit
git.run(&["checkout", "-b", "branch1", "master"])?;
git.commit_file("another_file", 2)?;

// Run smartlog with --show-signature flag
let (stdout, _) = git.branchless("smartlog", &["--show-signature"])?;

// Simply verify the command runs without error
assert!(!stdout.is_empty(), "Smartlog output should not be empty");

Ok(())
}
Comment on lines +847 to +867
Copy link
Collaborator

Choose a reason for hiding this comment

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

[please change] This seems to be very similar to the previous test. Can you double to check and confirm that they're both really needed?


#[test]
fn test_show_signature_argument_parsing() -> eyre::Result<()> {
let git = make_git()?;
git.init_repo()?;

// Test that invalid flag combination reports error
// For example, combine with a flag that doesn't make sense (if any)
// If there are no incompatible flags, we can at least test that the flag is recognized

// Test help output includes the flag
let (help_stdout, _) = git.branchless("smartlog", &["--help"])?;
assert!(
help_stdout.contains("--show-signature"),
"Help output should include --show-signature option"
);

// Test that the flag is recognized (should execute without error)
let result = git.branchless("smartlog", &["--show-signature"]);
assert!(
result.is_ok(),
"The --show-signature flag should be recognized"
);

// Test that the flag works when combined with other flags
let result = git.branchless("smartlog", &["--show-signature", "--exact"]);
assert!(
result.is_ok(),
"The --show-signature flag should work with --exact"
);

let result = git.branchless("smartlog", &["--show-signature", "--reverse"]);
assert!(
result.is_ok(),
"The --show-signature flag should work with --reverse"
);

Ok(())
}
Comment on lines +869 to +906
Copy link
Collaborator

Choose a reason for hiding this comment

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

[please change] I also think that this test can be removed; mostly it seems to be testing that clap is working as expected. If I'm misunderstanding, let's update it to be more clear about which flags may or may not work with the new --show-signature flag.