372886 - test_bug351601.html does not throw, but does not test either

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Ray Kiddy]

This is a very simple test case. The bug says that, at one time, if the build ID was not set, then reading the navigator.buildID ivar from JavaScript would cause a crash.

So, this test verifies that this call cannot crash.

But this test, being part of the automated harness, will run only in environments where the build ID is set. There are no plans, AFAIK, to regularly build an app that does not have a build ID and then run the test suite on it.

So, this test should check. If the build ID is set and this call does not throw, it should call todo(). It calls ok() right now. It should only do that if there build ID is not set.

To claim that the test passes when the build ID is set is a false positive result.

Comment 1

[Ray Kiddy]

Attachment: patch to fix bug, todo if in production build

Comment 2

[Nickolay_Ponomarev]

Did you forget to ask for review?

I don't understand why you make this todo(), it looks like it just doesn't need to be tested in production builds.

Comment 3

[Ray Kiddy]

Attachment: updated, turning the todo() into an innocuous ok()

I changed the todo() to an innocuous ok(). When the test is run on production builds, it definitely should pass.

Comment 4

[Nickolay_Ponomarev]

Set the r? to sayrer. And I think setting isOK to false in the catch block and using the ternary operator instead of the if would be slightly better.

Comment 5

[Nickolay_Ponomarev]

Checking in test_bug351601.html;
/cvsroot/mozilla/testing/mochitest/tests/test_bug351601.html,v  <--  test_bug351601.html
new revision: 1.2; previous revision: 1.1
done