Assert when a mochitest manifest entry is 'not a valid test'

NEW
Unassigned

Status

4 years ago
4 years ago

People

(Reporter: jgilbert, Unassigned)

Tracking

(Depends on: 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
I had tests that were not running because the files started were like `test-foo.html` instead of `test_foo.html`, and the only message was a warning buried in the mochitest spew about them 'not being valid tests'.

It should assert.
(Reporter)

Updated

4 years ago
See Also: → bug 1087560
would you want this as a 'test-unexpected-fail' in the log file?  how would you like the notification to be shown?
Flags: needinfo?(jgilbert)
(Reporter)

Comment 2

4 years ago
(In reply to Joel Maher (:jmaher) from comment #1)
> would you want this as a 'test-unexpected-fail' in the log file?  how would
> you like the notification to be shown?
No, this is not a test error, it's a manifest error. Just throw an assert from python.
Flags: needinfo?(jgilbert)
this failure comes from inside the browser (javascript) where the test definition is.  I would like to avoid duplicating the logic here, but I could duplicate it.  We have different rules for chrome, browser-chrome, plain.
Sadly at the moment I think we still have junk in our manifests (from when we auto-converted from Makefiles) that means we can't make this fatal yet.
(Reporter)

Comment 5

4 years ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #4)
> Sadly at the moment I think we still have junk in our manifests (from when
> we auto-converted from Makefiles) that means we can't make this fatal yet.

I'm willing do to the mechanical patches to fix this.
That's bug 983867. I am happy to review patches!
Depends on: 983867
(Reporter)

Updated

4 years ago
Depends on: 1089029
(Reporter)

Updated

4 years ago
Depends on: 1089030
(Reporter)

Updated

4 years ago
Depends on: 1089031
(Reporter)

Updated

4 years ago
Depends on: 1089032
(Reporter)

Updated

4 years ago
Depends on: 1089033
(Reporter)

Updated

4 years ago
Depends on: 1089034
(Reporter)

Updated

4 years ago
Depends on: 1089035
(Reporter)

Updated

4 years ago
Depends on: 1089036
(Reporter)

Updated

4 years ago
Depends on: 1089037
:vaibhav, there are a lot of dependencies here, this is additional manifest work you could easily help out on!
(Reporter)

Comment 8

4 years ago
I'd like to note that while it's easy to just remove these, since they're already not being run, a bunch of them look like valid tests. A module owner/peer should check that we can just remove them.
You need to log in before you can comment on or make changes to this bug.