1039172 - Warnings are printed twice and "TEST-SKIPPED" are no longer printed in logs

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Vaibhav (:vaibhav1994)]

"Warnings:" are printed twice in the logs as which can be seen here: https://tbpl.mozilla.org/php/getParsedLog.php?id=43902177&tree=Try&full=1

Also "TEST-SKIPPED" are no longer printed in the logs.

Comment 1

[Vaibhav (:vaibhav1994)]

I think that printing "Warnings:" are not used anywher

Comment 2

[Vaibhav (:vaibhav1994)]

I think that "Warnings:" are being printed but are not used anywhere. Similarly the "TEST-SKIPPED" are not used anywhere else people would have raised problems. Joel, do you think we should stop printing "Warnings:" and "TEST-SKIPPED" and make the logs shorter?

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

so test-skipped is useful while figuring out what ran and didn't run.  Until we have a tool which allows us to see what tests are active/disabled per platform per branch, we should leave the skipped in place.

As for the warnings, we really should figure that out, for example here is a warning:
http://dxr.mozilla.org/mozilla-central/source/content/base/test/mochitest.ini#322

that is a resource for a test, not a test, so our manifest files could use some cleanup.  Lets make a pass through and see if we can get rid of these warnings.

could the warnings be printed twice because we are parsing the manifests twice?

Comment 4

[Vaibhav (:vaibhav1994)]

Sorry, wrong bug!

Comment 5

[Vaibhav (:vaibhav1994)]

(In reply to Joel Maher (:jmaher) from comment #3)
> so test-skipped is useful while figuring out what ran and didn't run.  Until
> we have a tool which allows us to see what tests are active/disabled per
> platform per branch, we should leave the skipped in place.
The test-skipped are not being printed because we filter out the disabled tests for bisection here: http://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#1387 . So, one way is to log the test-skipped information there, but then it will be printed before ""runtests.py | Running tests: start.\n". Will this be fine or is there some other way?


> As for the warnings, we really should figure that out, for example here is a
> warning:
> http://dxr.mozilla.org/mozilla-central/source/content/base/test/mochitest.
> ini#322
> 
> that is a resource for a test, not a test, so our manifest files could use
> some cleanup.  Lets make a pass through and see if we can get rid of these
> warnings.

Should I remove the warning log here: http://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#1363 ? since we already have all the warning failures in https://bugzilla.mozilla.org/show_bug.cgi?id=1039539#c1

 
> could the warnings be printed twice because we are parsing the manifests
> twice?
Yes, that is the reason.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

I am fine with printing the skipped information elsewhere, as long as it is only once!


My vote is to keep the warning log message, this will get cleaned up and we will be ignoring incorrect manifests if we remove this.

Comment 7

[Vaibhav (:vaibhav1994)]

Attachment: testskipped.patch

This patch puts the "TEST-SKIPPED" in place.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8458687 [details] [diff] [review]
testskipped.patch

Review of attachment 8458687 [details] [diff] [review]:
-----------------------------------------------------------------

you are awesome!

Comment 9

[Vaibhav (:vaibhav1994)]

Thanks Joel for the quick review :)

Comment 10

[(Out Sep 17-23) Ryan VanderMeulen]

https://hg.mozilla.org/integration/mozilla-inbound/rev/51b6fd0998d8

Comment 11

[(Out Sep 17-23) Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/51b6fd0998d8

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

I would like to close this bug out or be explicit about what is left to do.  quite possible remove the test-skipped messages now that we have test-informant?

Comment 13

[Vaibhav (:vaibhav1994)]

Yes, we can close this as the remaining part will be dealt in Bug 983867

Comment 14

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.