Bug 2064655 - Add test coverage for live policies that simplify debugging by 1rneh · Pull Request #1302 · mozilla/enterprise-firefox

1rneh · 2026-08-19T10:34:09Z

Description

Bugzilla: Bug-2064655

Adds shared helper waitForLivePolicyUpdate and refactors existing live tests to use it.
Adds test coverage for the live support of the policies BlockAboutConfig and BlockAboutSupport, DisableDeveloperTools

Dependencies

Bug-2028825; 9252d3e needs to land in upstream first.

Testing

Added tests
Manual testing performed

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

lissyx · 2026-08-19T16:47:22Z

This file should get feedback from devtools people IMHO

Good idea! I just asked for a review in the devtools channel on slack.

lissyx · 2026-08-19T19:14:36Z

+  info("Removing BlockAboutConfig");
+  await waitForLivePolicyUpdate({});
+  await checkBlockedPage(ABOUT_CONFIG_PAGE, false);


For what it is worth https://searchfox.org/enterprise-main/rev/0e9eb88ea8645c4e0d00cdcbe9b561131f41b8d3/testing/enterprise/test_felt_browser_about_config_blocked.py#22-47 we have more transitions in this test because I remember finding issues transitionning from arbitrary states to others.

Things have moved on the engine so maybe this is not relevant anymore, but maybe we want extra coverage here?

Ah, I didn't see test_felt_browser_about_config_blocked.py. I'd like to replace it with this one (but I'll wait until we have browser tests enabled) as I think the new browser test covers everything:

no policy

update to true (block)

update to false = explicit allow (no block)

remove

What the marionette tests additionally, is covering the reverse order - running step 3 followed by step 2 in the beginning.
One thing that has changed in the policy engine is that on a policy update we'll clear the previous state by running the onRemove callback first followed by the activation callback. So reversing the update order has no effect, as each update step starts with clearing any previous state.

lissyx · 2026-08-19T19:18:06Z

+      // The policy locks the pref to true and disables every entry point:
+      // checkDevToolsState asserting the keyset is removed, the menu items are
+      // hidden, and the feature is disallowed is what deterministically proves
+      // DevTools can no longer be opened.
+      await checkDevToolsState(browser, { locked: true, disabled: true });
+      await checkDevToolsAboutPages(true);


why do not we test await checkCanOpenDevTools(browser); here? Well a variant "cannot" but we should verify it no?

I tend also to remember some discussion with devtools folks around the fact that setting the policy may (or should?) close any opened devtool window, is this something we should test maybe?

(from my memory it was left "for the future" because of limitations in the devtools code handling, but I think it is part of what you change)

Good idea! A live policy update to DisableDeveloperTools:true should close any open devtool windows!

why do not we test await checkCanOpenDevTools(browser); here? Well a variant "cannot" but we should verify it no?

I don't know how to verify the absence of the devtools without a timeout. And I'd like to avoid introducing timeout flakiness.

lissyx

I have had a look at the copilot comment on gBrowser and I see similar code to yours accross the codebase so I'm not sure how much it's correct ...

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

toolkit/components/enterprisepolicies/tests/browser/live/head.js:69

Consider awaiting tab removal here (i.e., wait for the tab close to complete) to avoid test interference from a still-tearing-down tab/session history load, especially since this helper is intended for reuse across many tests.

  BrowserTestUtils.removeTab(newTab);

devtools/startup/DevToolsStartup.sys.mjs:401

A new pref observer is added here. To keep observer lifecycles balanced and prevent duplicate notifications/leaks across shutdown/reload paths, please add the matching Services.prefs.removeObserver(DEVTOOLS_POLICY_DISABLED_PREF, this.onDisabledByPolicyChanged) in the existing cleanup/uninit path for DevToolsStartup (where other observers are removed).

      if (AppConstants.MOZ_ENTERPRISE) {
        // React to the DisableDeveloperTools enterprise policy updates
        Services.prefs.addObserver(
          DEVTOOLS_POLICY_DISABLED_PREF,
          this.onDisabledByPolicyChanged
        );
      }

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (2)

toolkit/components/enterprisepolicies/tests/browser/live/head.js:42

The comment says the blocked page is replaced with about:neterror?e=blockedByPolicy, but this helper asserts on blockedByPolicyEnterprise. Keeping the comment in sync with the actual enterprise neterror code will avoid confusion when debugging failures.

// Navigate to `url` in a new tab and assert whether it was blocked by policy
// (replaced with about:neterror?e=blockedByPolicy) or loaded normally.

devtools/client/framework/browser-toolbox/Launcher.sys.mjs:90

BrowserToolboxLauncher.closeAll() calls the async launcher.close() without awaiting or handling rejections. If close() throws (e.g. process already exited), this can surface as an unhandled promise rejection when the policy toggles live. Consider catching/logging errors for each close attempt.

  static closeAll() {
    if (!AppConstants.MOZ_ENTERPRISE) {
      return;
    }
    for (const launcher of [...processes]) {
      launcher.close();
    }
  }

Observe devtools.policy.disabled at runtime so the policy no longer needs a restart. On change, DevToolsStartup removes/restores the key shortcuts, closes open in-window toolboxes and any Browser Toolbox (closeAll), and devtools-browser hides/shows the Tools-menu items. gDevTools.showToolbox also refuses to open while disabled, so the policy holds via any entry point. Differential Revision: https://phabricator.services.mozilla.com/D320404

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

+// Navigate to `url` in a new tab and assert whether it was blocked by policy
+// (replaced with about:neterror?e=blockedByPolicy) or loaded normally.


+  static closeAll() {
+    for (const launcher of [...processes]) {
+      launcher.close();
+    }
+  }


1rneh added 3 commits August 19, 2026 12:24

Bug 2064655 - Add helper method waitForLivePolicyUpdate

9a7be94

Bug 2064655 - Add live support testing for policy BlockAboutConfig

fa5010b

Bug 2064655 - Add live support testing for policy BlockAboutSupport

ec0e8d4

1rneh requested a review from a team August 19, 2026 10:34

1rneh self-assigned this Aug 19, 2026

Copilot AI lite review requested due to automatic review settings August 19, 2026 10:34

1rneh added the branch:main PR that should be merged on enterprise-main branch label Aug 19, 2026

Copilot started reviewing on behalf of 1rneh August 19, 2026 10:34 View session

Copilot AI reviewed Aug 19, 2026

1rneh requested a lite review from Copilot August 19, 2026 10:57

Copilot started reviewing on behalf of 1rneh August 19, 2026 10:58 View session

Copilot AI reviewed Aug 19, 2026

View reviewed changes

Comment thread toolkit/components/enterprisepolicies/tests/browser/live/head.js

lissyx reviewed Aug 19, 2026

View reviewed changes

Comment thread devtools/startup/DevToolsStartup.sys.mjs Outdated

lissyx reviewed Aug 19, 2026

View reviewed changes

lissyx approved these changes Aug 19, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 20, 2026 16:19

Copilot AI reviewed Aug 20, 2026

View reviewed changes

Comment thread toolkit/components/enterprisepolicies/tests/browser/live/head.js

Comment thread devtools/client/framework/browser-toolbox/Launcher.sys.mjs Outdated

Copilot AI review requested due to automatic review settings August 20, 2026 16:47

1rneh force-pushed the bug-2064655-live-support-debugging-policies branch from 85f1eac to 1b5b41a Compare August 20, 2026 16:47

Copilot AI reviewed Aug 20, 2026

View reviewed changes

Copilot started reviewing on behalf of 1rneh August 20, 2026 19:11 View session

Copilot started reviewing on behalf of 1rneh August 20, 2026 19:35 View session

Copilot stopped reviewing on behalf of 1rneh due to an error August 20, 2026 20:07
Copilot had to stop work due to a timeout.

1rneh added 2 commits August 24, 2026 08:22

Bug 2064655 - Add live support testing for policy DisableDeveloperTools

70cb372

Copilot AI review requested due to automatic review settings August 24, 2026 06:44

1rneh force-pushed the bug-2064655-live-support-debugging-policies branch from 1b5b41a to 70cb372 Compare August 24, 2026 06:44

Copilot started reviewing on behalf of 1rneh August 24, 2026 06:45 View session

1rneh changed the title ~~Bug 2064655 - Add live support for policies simplifying debugging~~ Bug 2064655 - Add test coverage for live policies that simplify debugging Aug 24, 2026

Copilot AI reviewed Aug 24, 2026

View reviewed changes

		// Navigate to `url` in a new tab and assert whether it was blocked by policy
		// (replaced with about:neterror?e=blockedByPolicy) or loaded normally.

Uh oh!

Conversation

1rneh commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lissyx left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

1rneh commented Aug 19, 2026 •

edited

Loading