-
Notifications
You must be signed in to change notification settings - Fork 11
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
split fully qualified name into name + module path (reported as classname) #30
split fully qualified name into name + module path (reported as classname) #30
Conversation
junit-report v0.3 was released so this should build now |
Sorry I missed this @MalteSchledjewski ! I am adding some more tests (#31). Once that merges, could you merge and update tests as needed? This looks like a great contribution - I just want to be extra safe :) |
duration, | ||
exec_time, | ||
} => { | ||
let current_test_suite = current_suite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let current_test_suite = current_suite | |
let current_suite = current_suite | |
.as_mut() | |
.expect("Test event found outside of suite!"); |
@@ -137,16 +149,20 @@ fn parse<T: BufRead>( | |||
} | |||
TestEvent::Ok { name } => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TestEvent::Ok { name } => { | |
TestEvent::Ok { name } => { | |
assert!(tests.remove(&name)); | |
let (name, module_path) = split_name(&name); | |
*current_suite = current_suite.clone().add_testcase( | |
TestCase::success(&name, duration).set_classname(module_path.as_str()), | |
); | |
} | |
TestEvent::Failed { name, stdout } => { | |
assert!(tests.remove(&name)); | |
let (name, module_path) = split_name(&name); | |
*current_suite = current_suite.clone().add_testcase( | |
TestCase::failure(&name, duration, "cargo test", &stdout) | |
.set_classname(module_path.as_str()), | |
); | |
} |
meh I can just make the tweaks myself :) Thanks for your contribution! @MalteSchledjewski Can you try this out E2E in your scenario with:
If that looks good, I'll push to crates.io |
591339a works like a charm |
Thanks @MalteSchledjewski I just published: |
Could you set the |
@darxriggs I made an issue over here: #34 Is this something you'd want to contribute? I use Azure DevOps Pipelines so I'm unable to have a fast inner loop in testing it out. |
I am sorry to say that for the foreseeable future I have no plans to work on this. Currently I am using a simple workaround by just removing the empty |
GitLab apparently needs a
classname
attribute.I propose that the fully qualified name is split at the last
::
into{classname}::{name}
This requires bachp/junit-report-rs#5 to be merged.