-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix jenkins cobertura coverage #2346
Comments
So one thing I've found is that if I remove the I.e. the current response fails .reply(200, {
results: {
elements: [
{
name: 'Conditionals',
ratio: 95.146,
},
{
name: 'Lines',
ratio: 63.745,
},
],
},
}) but the same response object with just the one .reply(200, {
results: {
elements: [
{
name: 'Lines',
ratio: 63.745,
},
],
},
}) |
When I hit a live instance of the API with the same query params Shields uses, I get the following JSON response with various objects in the {
"_class":"hudson.plugins.cobertura.targets.CoverageResult",
"results":{
"elements":[
{
"name":"Packages",
"ratio":92.47312
},
{
"name":"Files",
"ratio":90.55556
},
{
"name":"Classes",
"ratio":90.94693
},
{
"name":"Methods",
"ratio":77.76264
},
{
"name":"Lines",
"ratio":74.3159
},
{
"name":"Conditionals",
"ratio":65.21124
}
]
}
} And i also see that the |
Ooh, thanks for looking into this! I see. Yea, I think the thing to do is change that Then we can filter the results in the handler. |
That's what I was thinking as well. I'll happily take this one and implement that change |
I didn't want to open a new issue for this (yet) but as an FYI.. The live test I added failed in the most recent daily build but i'm positive this was simply due to the test taking longer than the mocha timeout period of 5 seconds. I've seen this a couple times when I run the test locally where it occasionally takes 5-7 seconds. I'll try to keep an eye on this test and if it keeps intermittently failing I can try to find a different live jenkins endpoint that will hopefully respond more quickly, unless there's a possibility of increasing the mocha timeout period for the service tests in the daily build? I think this particular test would be fine as is with a timeout period of 7500ms vs. the current 5000ms
|
@calebcartwright you can increase the timeout on a per test basis. Simply add |
Excellent! Semi-random question for you all.. do you care if I add on a change like this to a different, but existing, PR I already have open (say #2477 ), or do you prefer separate PRs for different changes? |
I would recommend opening a new PR, as the currently opened one is for an unrelated service. 😉 |
Gotcha, thanks @PyvesB! |
This may be a legitimate failure. If nothing else, it should prompt a change to the error handling.
Ref: #1359
The text was updated successfully, but these errors were encountered: