1308722 - plugin is null: mail/test/mozmill/content-tabs/test-plugin-blocked.js: assert checks it but in the wrong place (too late)

Status: RESOLVED DUPLICATE of bug 1307516

(Thunderbird :: Testing Infrastructure, defect)

Description

[ISHIKAWA, Chiaki]

Attachment: check |plugin| bein null before accessing |plugin.enabledState|

I noticed four instances of 
  EXCEPTION: plugin is null
JavaScript error during local |make mozmill| test suite run of C-C TB.
One example is below:

SUMMARY-UNEXPECTED-FAIL | test-plugin-blocked.js | test-plugin-blocked.js::test_blocklisted_plugin_notification
  EXCEPTION: plugin is null
    at: test-plugin-blocked.js line 60
       test_blocklisted_plugin_notification test-plugin-blocked.js:60 7
       Runner.prototype.wrapper frame.js:585 9
       Runner.prototype._runTestModule frame.js:655 9
       Runner.prototype.runTestModule frame.js:701 3
       Runner.prototype.runTestDirectory frame.js:525 7
       runTestDirectory frame.js:707 3
       Bridge.prototype._execFunction server.js:179 10
       Bridge.prototype.execFunction server.js:183 16
       Session.prototype.receive server.js:283 3
       AsyncRead.prototype.onDataAvailable server.js:88 3



Upon checking the code, 
I found |plugin| was null when it was referenced in the following code.

  function test_blocklisted_plugin_notification() {
  let plugin = get_test_plugin();
->let pluginState = plugin.enabledState;
  plugin.enabledState = plugin.STATE_ENABLED;
  assert_not_equals(plugin, null, "Test plugin not found");
  assert_false(plugin.blocklisted, "Test plugin was unexpectedly blocklisted");
  assert_false(plugin.disabled, "Test plugin not enabled");

A few lines below, there is
  assert_not_equals(plugin, null, "Test plugin not found");
which obviously checks this condition, but it comes too late.

Moving the check before referecing plugin.enabledState made the errors disappear. 

I have no idea whether that is the intended behavior.

Patch attached.

Comment 1

[Jorg K (CEST = GMT+2)]

(In reply to ISHIKAWA, Chiaki from comment #0)
> I noticed four instances of 
>   EXCEPTION: plugin is null

We noticed them, too, and we fixed them. Bug 1307516.

BTW, your patch isn't right ;-(

Comment 2

[ISHIKAWA, Chiaki]

(In reply to Jorg K (GMT+2) from comment #1)
> (In reply to ISHIKAWA, Chiaki from comment #0)
> > I noticed four instances of 
> >   EXCEPTION: plugin is null
> 
> We noticed them, too, and we fixed them. Bug 1307516.
> 

Thank you.

> BTW, your patch isn't right ;-(
> 

Right, like I said "I have no idea whether that is the intended behavior."
I had no idea this was related to NAPI and other plugin issues.

> *** This bug has been marked as a duplicate of bug 1307516 ***

Anyway, great to see a bug is being fixed.
I will file another bug concerning the timeout (on local machine testing)
of composition/test-attachment-reminder.js.
I just realized (maybe too late) that this has a grave consequence for local testing. 
(No tests that follow in the composition subdirectory get executed after the timeout.)
But I digress.

Comment 3

[Jorg K (CEST = GMT+2)]

(In reply to ISHIKAWA, Chiaki from comment #2)
> I will file another bug concerning the timeout (on local machine testing)
> of composition/test-attachment-reminder.js.
Isn't that bug 1070665? You've already looked into it, right?

Comment 4

[ISHIKAWA, Chiaki]

(In reply to Jorg K (GMT+2) from comment #3)
> (In reply to ISHIKAWA, Chiaki from comment #2)
> > I will file another bug concerning the timeout (on local machine testing)
> > of composition/test-attachment-reminder.js.
> Isn't that bug 1070665? You've already looked into it, right?

Right. I uploaded some screendumps there to explain what I see during test. Thanks!