1628188 - browser/extensions/formautofill/test/unit/ tests fail in unpackaged builds with NS_ERROR_NOT_AVAILABLE when importing from resource://formautofill/

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, defect, P1)

Description

[Matthew N. [:MattN]]

STR:

1. ./mach -v xpcshell-test browser/extensions/formautofill/test/unit/test_createRecords.js

Expected result:

• The test passes

Actual result:

0:02.36 ERROR Unexpected exception NS_ERROR_NOT_AVAILABLE:
seutp@/Users/matthew/mozilla-central/obj-fx-opt/_tests/xpcshell/browser/extensions/formautofill/test/unit/test_createRecords.js:9:42
_run_next_test/<@/Users/matthew/mozilla-central/testing/xpcshell/head.js:1567:22
_run_next_test@/Users/matthew/mozilla-central/testing/xpcshell/head.js:1567:38
run@/Users/matthew/mozilla-central/testing/xpcshell/head.js:735:9
_do_main@/Users/matthew/mozilla-central/testing/xpcshell/head.js:246:6
_execute_test@/Users/matthew/mozilla-central/testing/xpcshell/head.js:573:5
@-e:1:1

which refers to the line with: … = ChromeUtils.import("resource://formautofill/FormAutofillHandler.jsm"));

The same applies to most other tests in the directory… they seem to fail on lines that try to import from JSMs under resource://formautofill/ but only when run from object directories, not in automation. The workaround is to do a ./mach package step, extract the package, and then run the test referencing the extracted package contents, example for macOS:

./mach -v xpcshell-test browser/extensions/formautofill/test/unit/test_createRecords.js --app-path /Volumes/Nightly/Nightly.app/Contents/Resources/browser --xre-path /Volumes/Nightly/Nightly.app/Contents/Resources

I would guess this has to do with it being a built-in system add-on. We already have some code that I think it supposed to handle packaged vs. unpackaged builds (I forget the proper terminology) but I'm guessing it needs updating or isn't sufficient anymore.

A regression range using an artifact build and hg bisect would pinpoint the problem.

Comment 1

[Matthew N. [:MattN]]

hg bisect points to bug 1524327:

changeset 572546:85753283ef2e: bad
The first bad revision is:
changeset: 572546:85753283ef2e
user: Shane Caraveo <[e-mail]>
date: Wed Jan 15 21:38:40 2020 +0000
summary: Bug 1524327 remove MOZ_ALLOW_LEGACY_EXTENSIONS and extensions.legacy.enabled r=zombie,aswan

Comment 2

[Andrew Swan [:aswan]]

Dropping this line next the top of the formautofill head.js allows the test to pass for me:

Services.prefs.setBoolPref("extensions.experiments.enabled", true);

I don't have time at the moment to investigate further but I'm not sure why it would work with packaged builds or how it was working prior to bug 1524327 (that bug changed the name of the preference needed to enable experiments, but similar code to set extensions.legacy.enabled would have been needed prior)

Comment 3

[Daniel Bodea [:danibodea]]

Considering this bug has the "regressionwindow-wanted" tag, it falls in our regressor investigation bug list, but I see it also has the "qe-verify-" tag.
Does this mean we should not attempt to find it's regressor? I am thinking this might be impossible through manual testing, right?

Comment 4

[Matthew N. [:MattN]]

I already found the regression window in comment 1 but forgot to remove the keyword. In this case the regression is in the tooling, not in Firefox, so I wouldn't expect QA to be the ones to find the window.

Comment 5

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Hi, this is a bug whose current Severity is blocker but needs to be updated for the new Severity values as of May 4 2020.

I am moving its severity to S1.

Please review this bug's Severity and let Release Management know if it still is a high Severity bug.

Comment 6

[Pascal Chevrel:pascalc]

Marking as S3 as it doesn't affect end users.

Comment 7

[Adam Roach [:abr]]

(In reply to Andrew Swan [:aswan] from comment #2)

Dropping this line next the top of the formautofill head.js allows the test to pass for me:

Services.prefs.setBoolPref("extensions.experiments.enabled", true);

I don't have time at the moment to investigate further but I'm not sure why it would work with packaged builds or how it was working prior to bug 1524327 (that bug changed the name of the preference needed to enable experiments, but similar code to set extensions.legacy.enabled would have been needed prior)

If I read it correctly, I think the "similar code" that was removed is these 12 lines, which appear to turn the corresponding environment variable on if milestone.is_nightly is set: https://hg.mozilla.org/integration/autoland/rev/85753283ef2e#l35.12

Comment 8

[Adam Roach [:abr]]

Attachment: Bug 1628188: Fix formautofill unit tests r?zbraniecki

Comment 9

[Matthew N. [:MattN]]

(In reply to Adam Roach [:abr] from comment #7)

(In reply to Andrew Swan [:aswan] from comment #2)

Dropping this line next the top of the formautofill head.js allows the test to pass for me:

Services.prefs.setBoolPref("extensions.experiments.enabled", true);

I don't have time at the moment to investigate further but I'm not sure why it would work with packaged builds or how it was working prior to bug 1524327 (that bug changed the name of the preference needed to enable experiments, but similar code to set extensions.legacy.enabled would have been needed prior)

If I read it correctly, I think the "similar code" that was removed is these 12 lines, which appear to turn the corresponding environment variable on if milestone.is_nightly is set: https://hg.mozilla.org/integration/autoland/rev/85753283ef2e#l35.12

So do we not want to fix this then rather than working around it? We can maybe land the workaround but keep this bug open for the proper fix?

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e4fa9e3ccea
Fix formautofill unit tests r=zbraniecki

Comment 11

[Adam Roach [:abr]]

(In reply to Matthew N. [:MattN] from comment #9)

So do we not want to fix this then rather than working around it? We can maybe land the workaround but keep this bug open for the proper fix?

It's not clear that reverting the moz.build change is what's called for here: it looks like removing this behavior was intentional. Shane -- is there any reason we shouldn't revert this specific change, with appropriate renaming from MOZ_ALLOW_LEGACY_EXTENSIONS to EXPERIMENTS_ENABLED?

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/3e4fa9e3ccea

Comment 13

[Shane Caraveo (:mixedpuppy)]

(In reply to Adam Roach [:abr] from comment #7)

(In reply to Andrew Swan [:aswan] from comment #2)

Dropping this line next the top of the formautofill head.js allows the test to pass for me:

Services.prefs.setBoolPref("extensions.experiments.enabled", true);

I don't have time at the moment to investigate further but I'm not sure why it would work with packaged builds or how it was working prior to bug 1524327 (that bug changed the name of the preference needed to enable experiments, but similar code to set extensions.legacy.enabled would have been needed prior)

If I read it correctly, I think the "similar code" that was removed is these 12 lines, which appear to turn the corresponding environment variable on if milestone.is_nightly is set: https://hg.mozilla.org/integration/autoland/rev/85753283ef2e#l35.12

A change later[1] should have handled this, I'd rather understand why that didn't effectively turn on experimental api support for the test, rather than reverting the build environment support.

[1] https://hg.mozilla.org/integration/autoland/rev/85753283ef2e#l36.31

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:abr, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.