983867 - mochitest.ini files contain lots of non-tests

Status: NEW

(Testing :: Mochitest, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: mochitest-plain issues

While working on bug 938019, I found that there appear to be a lot of things that aren't tests listed as tests in mochitest.ini files. Usually they have a name like test_something.js, which is rejected by the server.js code. I'm attaching a complete list of these for mochitest-plain.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: chrome issues

Here are the problems with chrome tests.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

browser-chrome only has one issue:

Error: browser_637020_slow.sjs from manifest /home/billm/mozilla/in1/objdir-ff-dbg/_tests/testing/mochitest/browser/browser/components/sessionstore/test/browser.ini is not a valid test

Comment 3

[(not currently active) Ted Mielczarek]

It's entirely possible that these are just things I screwed up in translation. Sorry about that!

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Note: see https://bugzilla.mozilla.org/show_bug.cgi?id=986163#c5 for a complication to this bug.

Comment 5

[Dan Minor [:dminor]]

I cooked up a little python script to patch the manifests in a semi-automated way and was able to clean up Linux M1 without too much trouble. I'll continue to look at this.

Comment 6

[Dan Minor [:dminor]]

Attachment: Remove some non-tests from mochitest manifests

Try run is here: https://tbpl.mozilla.org/?tree=Try&rev=e40158f03d6d

Unfortunately, a number of the non-tests are under 'imptests' and will need to be fixed upstream.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 8425509 [details] [diff] [review]
Remove some non-tests from mochitest manifests

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

::: dom/encoding/test/unit/moz.build
@@ +1,5 @@
>  # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
>  # vim: set filetype=python:
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.

Looks like you could remove this entire moz.build file, since it's empty now. (And remove it from wherever it's listed in DIRS.)

::: webapprt/test/content/mochitest.ini
@@ -4,5 @@
>    test.webapp
>    test.webapp^headers^
>  
> -[webapprt_sample.html]
> -[webapprt_indexeddb.html]

Huh, I wonder how these wound up in here if they're not used anywhere?

Comment 8

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #7)
> Comment on attachment 8425509 [details] [diff] [review]
> Remove some non-tests from mochitest manifests
> 
> ::: webapprt/test/content/mochitest.ini
> @@ -4,5 @@
> >    test.webapp
> >    test.webapp^headers^
> >  
> > -[webapprt_sample.html]
> > -[webapprt_indexeddb.html]
> 
> Huh, I wonder how these wound up in here if they're not used anywhere?

I found out a while ago that the webapp runtime tests kind of piggyback on mochitest manifests. They really should use their own manifests, but for some reason they don't. When someone tries to run webapp runtime tests, we act as if we're running mochitests, but we use a different filename filter. Anyway, I think that removing these lines will break webapp runtime tests. We don't run these on tinderbox now, but somebody will probably get irked if they're removed.

Comment 10

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

is there anything left here to sort out?  I am under the impression this is resolved - at the very least we should document in this bug what is left if anything.

Comment 11

[Dan Minor [:dminor]]

(In reply to Joel Maher (:jmaher) from comment #10)
> is there anything left here to sort out?  I am under the impression this is
> resolved - at the very least we should document in this bug what is left if
> anything.

The patch above fixed some things, but my try run was only for linux. I think some problems showed up on other platforms. I unassigned myself because I thought Vaibhav was going to mentor a contributor on this bug, but it looks like there was a miscommunication.

I guess we should see if we can resurrect my patch and land it, then file separate follow up bugs to clean up the webapprt tests and the upstreamed tests.

Comment 12

[Dan Minor [:dminor]]

It looks like everything here has subsequently been fixed except for tests under dom/imptests which are imported from an upstream w3c repository.