Characterize user-facing log messages #175
Conversation
src/lib.rs
Outdated
|
|
||
| let mut revwalk = ctx.repo.revwalk().unwrap(); | ||
| revwalk.push_head().unwrap(); | ||
| assert_eq!(revwalk.count(), 2); // git rebase wasn't called so both commits persist | ||
| let is_something_in_index = !nothing_left_in_index(&ctx.repo).unwrap(); | ||
| assert!(is_something_in_index); | ||
|
|
||
| log_utils::assert_log_messages_match( |
There was a problem hiding this comment.
Probably overkill. When I wrote it, I thought I'd be using it more. If we restrict ourselves to just checking the level and msg, this could go away.
There was a problem hiding this comment.
yeah i think this is too fussy for my tastes. let's not make future test writers jump through this hoop - the message alone is good enough
src/lib.rs
Outdated
| log_utils::assert_log_messages_are( | ||
| capturing_logger.visible_logs(), | ||
| vec
blairconrad
force-pushed
the
characterize-logging
branch
2 times, most recently
from
6294a8e to
65cf27a
Compare
|
I see some log-affecting PRs coming in. I don't think they'll conflict with this, but if they get merged, I'll assess and rebase. |
src/lib.rs
Outdated
|
|
||
| let mut revwalk = ctx.repo.revwalk().unwrap(); | ||
| revwalk.push_head().unwrap(); | ||
| assert_eq!(revwalk.count(), 2); // git rebase wasn't called so both commits persist | ||
| let is_something_in_index = !nothing_left_in_index(&ctx.repo).unwrap(); | ||
| assert!(is_something_in_index); | ||
|
|
||
| log_utils::assert_log_messages_match( |
There was a problem hiding this comment.
yeah i think this is too fussy for my tastes. let's not make future test writers jump through this hoop - the message alone is good enough
We're about to make some changes to log messages, so let's first see what we're logging to ensure we don't break anything.
blairconrad
force-pushed
the
characterize-logging
branch
from
65cf27a to
9a2e693
Compare
|
Thanks, @tummychow.
Was not aware of SlogValue until your comment. Meanwhile, I'm pushing in case you want to merge (but I don't mind being stuck behind #177; whatever's easier for you). |
|
haha, oops. I'd pushed without the log-checking in the extra test methods, then got called away. Came back, added them, and was about to push the updates. But I'm too late. Sorry. (In this case the extra log-checking doesn't really add anything; all the "all hunks turned into commits" emit about the same log messages, unless the stack is bounded by another author or a merge.) |
I've investigated and am not sure what you're looking for. What are you hoping will use SlogValue? I experimented with a future message like #[derive(Clone, ExtLoggable, Serialize)]
#[LogDetails(
Id = "101",
Text = "No changes staged, even after auto-staging. Try adding something to the index.",
Level = "WARN"
)]
struct NoChangesStaged;
...
if nothing_left_in_index(repo)? {
xlog!(ui_logger, NoChangesStaged);
return Ok(());
}And it may be something that will work, but I'm coming up with a panic when it runs. |
|
i don't really care about the statistics, but having typed logging objects would make the user-facing logs more consistent and testable. eg right now you have to write out the log message once at the callsite where it's logged, and again at the place where you're testing it. it would be better to get those things into a single place. if you can't get it working, that's fine, but i think it's worth an attempt |
|
I'll see what I can do. |
Contributes to #172, implementing