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

NEW
Unassigned

Status

defect
5 years ago
5 years ago

People

(Reporter: jgilbert, Unassigned)

Tracking

(Depends on 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

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.
See Also: → 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)
(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.
(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
Depends on: 1089029
Depends on: 1089030
Depends on: 1089031
Depends on: 1089032
Depends on: 1089033
Depends on: 1089034
Depends on: 1089035
Depends on: 1089036
Depends on: 1089037
:vaibhav, there are a lot of dependencies here, this is additional manifest work you could easily help out on!
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.