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

workaround an issue that causes function arguments to show up as if they are uncovered #1726

Merged
merged 2 commits into from
Jul 15, 2021

Conversation

alex
Copy link
Contributor

@alex alex commented Jul 15, 2021

rust-lang/rust#86972 describes this issue in some detail and links to a minimal reproducer

making this change, and this change alone, resolves the coverage issues

…hey are uncovered

rust-lang/rust#86972 describes this issue in some detail and links to a minimal reproducer

making this change, and this change alone, resolves the coverage issues
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Hmmm, interesting.

So if I understand correctly, we've generated a branch which spans the argument, however in your tests it's not covered. You (or any other user doing similar things) would have to invoke every argument with incorrect types to achieve full coverage.

I think it's reasonable to adjust this in this way. Can we add a CHANGELOG message please, in case others are also noticing this behaviour?

I am curious though: if branches like this are a problem, I wonder why you're not having issues with the branching around default arguments slightly further down the file?

let arg_value_or_default = match (spec.default_value(name), arg.optional.is_some()) {
(Some(default), true) if default.to_string() != "None" => {
quote_arg_span! { #arg_value.map_or_else(|| Ok(Some(#default)), |_obj| #extract)? }
}
(Some(default), _) => {
quote_arg_span! { #arg_value.map_or_else(|| Ok(#default), |_obj| #extract)? }
}
(None, true) => quote_arg_span! { #arg_value.map_or(Ok(None), |_obj| #extract)? },
(None, false) => {
quote_arg_span! {
{
let _obj = #arg_value.expect("Failed to extract required method argument");
#extract?
}
}
}
};

@alex
Copy link
Contributor Author

alex commented Jul 15, 2021

I have really no explanation for why it's not impacting all of these spans or anything like that.

I'll add the changelog entry.

@davidhewitt
Copy link
Member

👍 I guess we can always tweak further if other users find other cases. Thanks very much for fixing.

@davidhewitt davidhewitt merged commit 15a193d into PyO3:main Jul 15, 2021
@alex alex deleted the patch-1 branch July 15, 2021 22:37
@alex
Copy link
Contributor Author

alex commented Jul 15, 2021

Thanks for the quick merge!

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.

None yet

2 participants