Ai kill switch - #1140
Conversation
ben-c-at-moz
left a comment
ben-c-at-moz
left a comment
There was a problem hiding this comment.
Excellent first PR for StarFox tests. I know our system has some idiosyncrasies. Just a few things to look into at first, and if you want to merge a recently-pulled main into this branch and make sure your tests exist in manifests/key.yaml. You can use python scripts/addtests.py for that if you like.
| # Nightly might not have this element, that's okay | ||
| import time | ||
| time.sleep(1) # Give the page a moment to settle | ||
| pass |
There was a problem hiding this comment.
We do need this check to run when we're executing against Beta -- can we refactor this so that we simply ignore the check in Nightly instead of having a fallback? (The fallback invalidates the version check for Beta as well)
There was a problem hiding this comment.
We still need to refactor this so that we don't invalidate version testing for Beta -- but the current code on main is pretty different than what was changed here, I believe.
There was a problem hiding this comment.
Do we need this file in the repository?
| # Simulate keyboard interaction - in real test, use pynput | ||
| # For now, we verify the element is accessible | ||
| logging.info("AI Controls toggle is accessible") | ||
| except Exception as e: |
There was a problem hiding this comment.
we don't have to do explicit pytest.fail() calls--the failure message in the assert call will log as an error. If you want that additional info, you could add it to the failure message in the assert. This isn't a dealbreaker, and if you really want this kind of messaging you can keep it as is.
There was a problem hiding this comment.
Please separate this into three test files, one per test case in TestRail.
There was a problem hiding this comment.
Please split this into one file per test case
There was a problem hiding this comment.
Please break this out into one test file per test case
There was a problem hiding this comment.
Please break this out into one test file per test case
There was a problem hiding this comment.
Progress, but we need to resolve some code issues and some convention issues, and one versioning issue:
Versioning
- We need to rebase this branch on a current
main
Convention
- Every test file needs to have a
test_casefixture that returns the TestRail case number as a string - If there are multiple "concerns" covered by a single test case, they can be separate test functions, but those test functions need to be in the same test file
- The suite_id fixture in the suite conftest.py file should correspond to the suite number of the tests in TestRail, in the form
("Snnn", "Suite Description"), wherenis an integer. If the tests are in multiple suites, those tests should be in different subdirectories of./tests`. - Currently, we do not support subfolders of suites. We could implement this in future, but for now, all of these new tests should live directly in
tests/ai_controls(unless you need multiple suites, as mentioned above. If this is the case, they would be intests/ai_controls_a11y,tests/ai_controls_functional, etc.)
Code
- In general, we prefer
WebDriverWait().until()overassert. Many functions onBasePageimplementuntil(), likeexpect(),element_visible(),url_contains(), etc. - In some cases, an
assertor other testing code is in thetrybranch of atry: except Exception:block, which will prevent the tests from ever failing. - The code changes in the main
conftest.pyfile should be a separate PR. We currently do not run these tests against "true" Nightly, and aren't going to run into the concern addressed.
Let me know if you need any help addressing any of these points. Thanks for working on this!
| result: pass | ||
| splits: | ||
| - functional2 | ||
| ai_controls: |
There was a problem hiding this comment.
We may want a separate split called "ai_controls"...Not exactly sure on this one, will have to think about it.
There was a problem hiding this comment.
Need test_case fixture
There was a problem hiding this comment.
need test_case fixture
There was a problem hiding this comment.
need test_case fixture
There was a problem hiding this comment.
need test_case fixture
| try: | ||
| state = about_prefs.get_ai_translations_state() | ||
| logging.info(f"Translations feature state: {state}") | ||
| except Exception as e: |
There was a problem hiding this comment.
Test unlikely to fail as all exceptions caught on the main testing code.
| try: | ||
| toggle_state = about_prefs.get_ai_killswitch_state() | ||
| logging.info(f"Killswitch state under policy: {toggle_state}") | ||
| except Exception as e: |
There was a problem hiding this comment.
Test unlikely to fail as all exceptions to testing code caught.
| C3341237 - AI controls labels localized | ||
| """ | ||
| about_prefs.navigate_to_ai_controls() | ||
| about_prefs.verify_ai_controls_page_loaded() |
| chatbot = about_prefs.get_element("ai-control-sidebar-chatbot-select") | ||
| assert chatbot is not None, "Chatbot provider control should exist" | ||
| logging.info("Chatbot feature is localized") | ||
| except Exception as e: |
There was a problem hiding this comment.
Test unlikely to fail as main testing code exceptions always caught.
| translations = about_prefs.get_element("ai-control-translations-select") | ||
| assert translations is not None, "Translations control should exist" | ||
| logging.info("Translations feature is localized") | ||
| except Exception as e: |
There was a problem hiding this comment.
Test unlikely to fail as all exceptions on testing code caught.
Code ReviewOverviewThis PR adds an Critical Issues1. Manifest keys don't match file names (will break the skip system) All files in 2. Per the project convention (CLAUDE.md): "Each test file should contain one test function." The three keyboard navigation tests need to be in their own files to be independently registered and skippable in the manifest. 3. This file is missing from POM Issues4. Bare Silently swallowing all exceptions hides real failures. If 5. Imports belong at the top of the file. Also, 6.
Test Logic Issues7. Invalid state value in
8. Most tests are empty stubs marked as
Tests that are stubs should be marked Minor
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @pytest.fixture() | ||
| def suite_id(): | ||
| # TODO: Replace with the actual TestRail suite number, e.g. ("12345", "AI Controls") | ||
| return ("TODO", "AI Controls") |
There was a problem hiding this comment.
The TestRail suite number is still a "TODO" placeholder. This needs to be filled in with the real suite ID before merge — the driver fixture depends on suite_id for test reporting.
| provider | ||
| ) |
There was a problem hiding this comment.
Two issues here:
from time import sleepshould be at the top of the file with other imports, not inside a method body.sleep(0.5)is a fixed-duration wait — an anti-pattern in Selenium. The backend change you're waiting for should be detectable via a DOM condition. Useself.expect()with a lambda that checks the resulting pref or element state instead.
| logging.info("Block all button may use moz-toggle element") | ||
|
|
||
|
|
||
| def test_keyboard_navigation_to_translations_setting(about_prefs: AboutPrefs): |
There was a problem hiding this comment.
This file contains 4 test functions, violating the project convention of one test per file (CLAUDE.md). The three keyboard navigation functions below (test_keyboard_navigation_to_translations_setting, test_keyboard_navigation_to_link_preview_setting, test_keyboard_navigation_to_smart_tab_groups_setting) need to be moved to their own files so they can be independently registered in key.yaml and skipped per-platform.
|
|
||
| @pytest.fixture() | ||
| def test_case(): | ||
| return "3341325" |
There was a problem hiding this comment.
This file uses TestRail case "3341325" which is already claimed by test_c3341325_provider_selection_reflects.py. Duplicate test_case IDs cause reporting collisions. Additionally, this file has no entry in key.yaml. It should either be removed (the scaffold intent is covered by the other file) or given a unique ID and registered in the manifest.
Review: AI Kill Switch TestsOverview: Adds a new Critical Issues1. Duplicate TestRail IDs
Each file must return its own unique TestRail case ID. If these are genuinely subtests of the same case, they should share a test file with subtest functions (see 2. Tests That Are Pure Stubs (No Assertions)Several tests navigate to the page and log a message but perform no actual verification. These will always pass regardless of browser behavior:
Tests that claim to verify something but don't fully deliver:
Keyboard Tests Don't Test Keyboard NavigationTests named POM:
|
| @pytest.fixture() | ||
| def suite_id(): | ||
| # TODO: Replace with the actual TestRail suite number, e.g. ("12345", "AI Controls") | ||
| return ("TODO", "AI Controls") |
There was a problem hiding this comment.
The suite_id is a placeholder — this must be replaced with the real TestRail suite ID before merging. A "TODO" value here will break TestRail result reporting for every test in this suite.
|
|
||
| @pytest.fixture() | ||
| def test_case(): | ||
| return "3276002" |
There was a problem hiding this comment.
Duplicate TestRail ID: test_c3276002_ai_features_keyboard_accessible.py, test_c3276002_keyboard_nav_link_preview.py, and test_c3276002_keyboard_nav_smart_tab_groups.py all return the same "3276002". Each test file needs a unique case ID, or these should be collapsed into one file as subtests.
|
|
||
| Arguments: | ||
| provider: Provider name (e.g., "ChatGPT", "Claude", "Copilot") | ||
| """ | ||
| select_elem = self.get_element("ai-control-sidebar-chatbot-select") |
There was a problem hiding this comment.
Bare except Exception silently swallows all failures. If find_in_settings genuinely may not apply here, narrow this to the specific exception it can raise (e.g. TimeoutException) so real errors like wrong context or a stale element aren't hidden.
| about_prefs.navigate_to_ai_controls() | ||
| about_prefs.verify_ai_controls_page_loaded() | ||
|
|
||
| toggle_state = about_prefs.get_ai_killswitch_state() | ||
| logging.info(f"Killswitch state under policy: {toggle_state}") | ||
| # Note: When DisableAIEnhancements policy is active, killswitch should be forced on. | ||
| # Full validation requires enterprise policy configuration in the browser profile. |
There was a problem hiding this comment.
This test has no assertions — it will always pass. The note acknowledges the limitation but the test shouldn't be merged in this state. Either: configure a profile with the DisableAIEnhancements enterprise policy and assert get_ai_killswitch_state() is True, or mark it disabled in key.yaml until the infrastructure is ready. The same applies to test_c3277713, test_c3277714, test_c3277716, and test_c3298825.
| about_prefs.element_exists("ai-control-sidebar-chatbot-select") | ||
| about_prefs.set_ai_chatbot_provider("ChatGPT") | ||
| logging.info("Chatbot provider set via AI controls page") |
There was a problem hiding this comment.
Missing assertion after setting the provider. set_ai_chatbot_provider silently no-ops if the provider name doesn't match any option. Add a verification step:
| about_prefs.element_exists("ai-control-sidebar-chatbot-select") | |
| about_prefs.set_ai_chatbot_provider("ChatGPT") | |
| logging.info("Chatbot provider set via AI controls page") | |
| about_prefs.set_ai_chatbot_provider("ChatGPT") | |
| assert about_prefs.get_ai_chatbot_provider() == "ChatGPT", ( | |
| "Chatbot provider should reflect 'ChatGPT' after selection" | |
| ) |
| # Use the find-in-settings box to reveal the control if needed | ||
| self.find_in_settings("AI") | ||
| except TimeoutException as e: | ||
| logging.warning(f"find_in_settings('AI') raised: {e}") |
There was a problem hiding this comment.
Silently swallowing TimeoutException here is risky. If the AI Controls pane isn't actually visible, the next get_element("ai-controls-toggle") call will also time out with a much less descriptive error. Either remove the try/except (let the navigation handle visibility), or re-raise after logging if the element still can't be found.
| ) | ||
|
|
||
| if block != current_state: | ||
| # Set the pressed property directly (clicking doesn't work on moz-toggle) |
There was a problem hiding this comment.
Setting element.pressed via JS bypasses all real UI interactions. The comment says clicking doesn't work on moz-toggle, but test C3276002 ("AI features keyboard accessible") is specifically meant to verify keyboard accessibility. The POM helper using JS is fine for state setup, but the keyboard accessibility test should be testing actual keyboard interaction (Tab + Space/Enter), not delegating to this method.
| self.driver.execute_script( | ||
| "arguments[0].dispatchEvent(new Event('change', { bubbles: true }));", | ||
| select_elem | ||
| ) |
There was a problem hiding this comment.
set_ai_translations doesn't verify the state was actually applied (no expect() call), unlike set_ai_blocking. Consider adding:
| ) | |
| self.expect(lambda _: self.get_ai_translations_state() == state) | |
| return self |
| select.dispatchEvent(new Event('change', { bubbles: true })); | ||
| """, | ||
| select_elem, | ||
| provider |
There was a problem hiding this comment.
set_ai_chatbot_provider silently does nothing if provider doesn't match any option (the loop just falls through, then the change event fires with the old value). This will make test_c3341325 pass even if "ChatGPT" isn't in the dropdown. Consider adding a return value from JS to detect a no-match and raising an error.
|
|
||
| def verify_ai_controls_page_loaded(self) -> BasePage: | ||
| """ | ||
| Verify that the AI Controls page has loaded successfully by checking for required elements. |
There was a problem hiding this comment.
verify_ai_controls_page_loaded only checks three elements but the suite has five AI control elements (also ai-control-link-preview-select and ai-control-smart-tab-groups-select). Either check all, or rename the method to reflect the partial scope (e.g., verify_ai_controls_core_elements_visible).
| @@ -0,0 +1,21 @@ | |||
| import pytest | |||
| from selenium.webdriver import Firefox | |||
There was a problem hiding this comment.
Firefox is imported but only used as a type annotation on the driver parameter — this import isn't strictly needed since driver is injected by the fixture system. Remove or replace with from __future__ import annotations if purely for type hints.
| @pytest.fixture() | ||
| def policies_list(): | ||
| return { | ||
| "AIControls": { |
There was a problem hiding this comment.
navigate_to_ai_controls() defaults to verify=True, which calls element_visible("ai-control-translations-select") and element_visible("ai-control-sidebar-chatbot-select"). With this policy active, those controls are hidden — so verification will time out before the test assertions are even reached. Use navigate_to_ai_controls(verify=False) here (and in the other policy-hiding tests: test_c3277713, test_c3277714, test_c3277716, test_c3298825).
| options.binary_location = fx_executable | ||
|
|
||
| # Write enterprise policies.json if any policies were requested | ||
| policy_file: Path | None = None |
There was a problem hiding this comment.
policy_file is initialised inside the try block. If an exception is raised before this line (e.g., from the Options() or argument-setter calls above), the finally block at line 586 will hit a NameError on policy_file. Move policy_file = None to just before the try: to guarantee it's always bound.
| policy_file: Path | None = None | |
| policy_file: Path | None = None |
|
|
||
|
|
||
| def test_block_all_ai_features_option_persists(about_prefs: AboutPrefs): | ||
| """ |
There was a problem hiding this comment.
If the kill switch hides individual feature controls in the UI, set_ai_translations will call get_element("ai-control-translations-select") on a hidden element and time out. Confirm whether individual controls remain accessible (just disabled) when the kill switch is on, or whether this call needs to bypass the element lookup (e.g., set the pref directly via JS).
| def navigate_to_ai_controls(self) -> BasePage: | ||
| """ | ||
| Navigate to the AI Controls preference page. | ||
| """ | ||
| self.driver.get("about:preferences#ai") | ||
| def navigate_to_ai_controls(self, verify: bool = True) -> "AboutPrefs": |
There was a problem hiding this comment.
There are two definitions of navigate_to_ai_controls back-to-back — this looks like a bad merge. The first definition (no verify param, returns nothing) is dead code since Python silently uses the second one. Delete lines 452–456.
| def navigate_to_ai_controls(self) -> BasePage: | |
| """ | |
| Navigate to the AI Controls preference page. | |
| """ | |
| self.driver.get("about:preferences#ai") | |
| def navigate_to_ai_controls(self, verify: bool = True) -> "AboutPrefs": | |
| def navigate_to_ai_controls(self, verify: bool = True) -> "AboutPrefs": |
| """ | ||
| try: | ||
| # Use the find-in-settings box to reveal the control if needed | ||
| self.find_in_settings("AI") | ||
| except Exception as e: | ||
| logging.warning(f"find_in_settings('AI') raised: {e}") |
There was a problem hiding this comment.
The "docstring" is actually commented-out code wrapped in a triple-quoted string — this method has no real docstring and the dead code block will confuse future readers. Replace with a proper docstring and keep the commented logic as a regular # comment if needed.
| """ | |
| try: | |
| # Use the find-in-settings box to reveal the control if needed | |
| self.find_in_settings("AI") | |
| except Exception as e: | |
| logging.warning(f"find_in_settings('AI') raised: {e}") | |
| """ | |
| Set the sidebar chatbot provider by matching option label. | |
| Arguments: | |
| provider: The display label of the provider (e.g. "ChatGPT"). | |
| """ |
| unpack_archive(os.path.join("profiles", f"{use_profile}.zip"), profile_path) | ||
| options.profile = profile_path | ||
|
|
||
| options.set_preference("app.update.disabledForTesting", False) |
There was a problem hiding this comment.
This line was previously commented out and is now being added unconditionally. Setting app.update.disabledForTesting to False (i.e. not disabling updates) means Firefox may try to auto-update during test runs, which can interfere with tests. Was this intentional? If the intent is to disable updates during testing the value should be True; if it's truly not needed, remove this line rather than setting it to False.
| Set the AI Translations feature state. | ||
|
|
||
| Arguments: | ||
| state: "available" or "blocked" |
There was a problem hiding this comment.
The docstring for set_ai_translations lists "available" or "blocked" as valid values, but get_ai_translations_state returns "enabled", "removed", or "blocked". Additionally, test_c3310314 calls set_ai_translations("enabled"). The valid state strings are inconsistent — please align the docstrings and usages to a single set of accepted values.
| return {} | ||
|
|
||
|
|
||
| def test_enable_ai_features_when_policy_changed(about_prefs: AboutPrefs): |
There was a problem hiding this comment.
This test is named "enable when policy changed" but the policies_list fixture returns {} — the same as the default. The test only verifies baseline behavior (killswitch is off with no policy); it doesn't simulate a policy being changed or removed. Consider either renaming the test to reflect what it actually checks, or adding a companion scenario that starts with a blocking policy and then overrides it.
Review: AI Kill Switch (#1140)OverviewAdds an Bugs / Must-Fix
Minor Issues
What Works Well
|
Review NotesOverall: Good start on the AI Controls test suite. Several issues need addressing before this is ready to merge. Blocking issues
Code quality issues
|
| return self.driver.execute_script( | ||
| "return arguments[0].pressed;", | ||
| toggle | ||
| ) |
There was a problem hiding this comment.
JS returns undefined (→ Python None) if the element lacks a pressed property, making assert result is False pass incorrectly or fail with a confusing message. Consider adding a cast:
| ) | |
| return bool(self.driver.execute_script( | |
| "return arguments[0].pressed;", | |
| toggle | |
| )) |
| Set the AI Translations feature state. | ||
|
|
||
| Arguments: | ||
| state: "available" or "blocked" |
There was a problem hiding this comment.
Docstring says valid states are "available" or "blocked", but get_ai_translations_state() can return "enabled" and test_c3310314 calls this method with "enabled". Reconcile the valid value sets across setter, getter, and all callers.
| """ | ||
| const select = arguments[0]; | ||
| const provider = arguments[1]; | ||
| for (let option of select.options) { |
There was a problem hiding this comment.
select.options is a property of HTMLSelectElement, not moz-select (a custom element). This will likely be undefined, causing for...of to throw a TypeError and always return false. You may need to query the shadow DOM for moz-option children instead, e.g. select.querySelectorAll('moz-option').
| unpack_archive(os.path.join("profiles", f"{use_profile}.zip"), profile_path) | ||
| options.profile = profile_path | ||
|
|
||
| options.set_preference("app.update.disabledForTesting", False) |
There was a problem hiding this comment.
This line was previously commented out; unconditionally setting it to False enables auto-updates during all tests across every suite — not just AI Controls. Please confirm this is intentional and, if so, move this change to a dedicated conftest.py PR with an explanation.
|
|
||
| @pytest.fixture() | ||
| def suite_id(): | ||
| return ("71443", "AI Controls") |
There was a problem hiding this comment.
Per project convention, the TestRail suite number should carry the S prefix.
| return ("71443", "AI Controls") | |
| return ("S71443", "AI Controls") |
| return {} | ||
|
|
||
|
|
||
| def test_enable_ai_features_when_policy_changed(about_prefs: AboutPrefs): |
There was a problem hiding this comment.
This test only verifies the default state when no policy is present. The TestRail case ("Enable AI features when policy changed") requires launching with a restricting policy active, then simulating removal/change of that policy and confirming features become re-enabled. The current implementation doesn't cover that scenario at all.
|
|
||
| initial_state = about_prefs.get_ai_killswitch_state() | ||
|
|
||
| # Block AI, then restore to initial state (simulating a cancel action) |
There was a problem hiding this comment.
This doesn't test cancellation — it enables blocking then re-disables it programmatically. The test case title ("Cancel blocking from the Block AI Enhancements prompt") implies a confirmation dialog that the user cancels. If that dialog doesn't exist yet, mark this disabled in the manifest rather than shipping a test that asserts the wrong thing.
| """ | ||
| about_prefs.navigate_to_ai_controls() | ||
|
|
||
| about_prefs.set_ai_translations("enabled") |
There was a problem hiding this comment.
set_ai_translations docstring says valid values are "available" or "blocked" — "enabled" is only a value the getter returns. Calling the setter with "enabled" may silently write an invalid pref value and the subsequent expect could pass for the wrong reason (element already in that state). Use "available" here to match the setter's documented API.
| C3341235 - Translations feature localized | ||
| """ | ||
| about_prefs.navigate_to_ai_controls() | ||
| about_prefs.element_exists("ai-control-translations-select") |
There was a problem hiding this comment.
This only checks that the element exists — it doesn't verify anything about localization. The TODO in the next line describes what's actually needed. This test is registered as result: pass in the manifest; it should be disabled until the document.l10n.formatValue(...) assertion is implemented.
|
|
||
| def _get_version(driver: Firefox): | ||
| driver.get(ABOUT_FIREFOX) | ||
| version_el = driver.find_element(By.ID, "version") |
There was a problem hiding this comment.
Potential flakiness: find_element is called immediately after driver.get() with no wait for the element to be present. The about:firefox page may not have finished rendering #version by the time this runs. Suggest using WebDriverWait here like _fx_up_to_date does:
| version_el = driver.find_element(By.ID, "version") | |
| WebDriverWait(driver, 10).until( | |
| EC.visibility_of_element_located((By.ID, "version")) | |
| ) | |
| version_el = driver.find_element(By.ID, "version") |
| """Collect executables, but just the version number for Fx""" | ||
| if not os.environ.get("CI"): | ||
| return None | ||
| return collect_executables.main("-n") |
There was a problem hiding this comment.
The previous guard if not os.environ.get("CI"): return None was removed, which means collect_executables.main("-n") now runs in all environments including local dev. If that script fails or is slow outside CI, every local test run will be affected. Was this intentional?
| @@ -415,14 +421,8 @@ def pytest_sessionfinish(session): | |||
| raise OSError("Could not find TestRail credentials") | |||
|
|
|||
| tr_session = tri.testrail_init() | |||
There was a problem hiding this comment.
The if tr_session is None: return guard was removed. If tri.testrail_init() returns None, calling tri.collect_changes(tr_session, report) here will raise an AttributeError or TypeError instead of failing gracefully. The guard should be kept.
| tri.mark_results(tr_session, passes) | ||
| else: | ||
| logging.warning("No test results found.") | ||
| tri.mark_results(tr_session, passes) |
There was a problem hiding this comment.
tri.mark_results is now called unconditionally even when passes is empty/falsy. Previously there was a guard:
if passes:
tri.mark_results(tr_session, passes)
else:
logging.warning("No test results found.")If mark_results doesn't handle an empty collection gracefully, this could cause a silent failure or error.
| logging.info(f"Removed enterprise policies file {policy_file}") | ||
| except OSError: | ||
| pass | ||
| if not hard_quit and ("driver" in locals() or "driver" in globals()) and driver: |
There was a problem hiding this comment.
The hard_quit path previously called driver.service.stop() before returning — that cleanup is now gone entirely. With this change, hard_quit=True only skips driver.quit(), but the service process is never stopped, which may leave a dangling geckodriver process.
|
|
||
| if block != current_state: | ||
| self.driver.execute_script( | ||
| "Services.prefs.setStringPref('browser.ai.control.default', arguments[0]);", |
There was a problem hiding this comment.
Services.prefs is only available in chrome context. Other AboutPrefs methods that interact with chrome-privileged APIs use @BasePage.context_chrome (e.g. remove_search_engine at line 168). This call will fail in content context. Consider adding the decorator — or wrapping the execute_script call in with self.driver.context(self.driver.CONTEXT_CHROME):.
| # moz-toggle uses the 'pressed' property; cast to bool for safety | ||
| # in case the property is missing (JS returns undefined → Python None) | ||
| return bool(self.driver.execute_script( | ||
| "return arguments[0].pressed;", |
There was a problem hiding this comment.
Same chrome-context issue: driver.execute_script("return arguments[0].pressed;", toggle) on a moz-toggle custom element — accessing .pressed as a JS property on a shadow-DOM component typically works in content context, but the get_element() call that precedes it may fail if the element requires chrome context to locate. Worth verifying this works correctly in a content-context environment.
Review: AI Kill SwitchOverviewAdds a new
|
2 participants
Delete this block after confirming that you will do the following:
Relevant Links
Bugzilla: _
TestRail: _
Description of Code / Doc Changes
Leave a bullet-pointed list of changes you made.
Process Changes Required
Mark the relevant boxes, delete irrelevant lines.
pipenv install)./devsetup.sh)Screenshots or Explanations
If you need to explain your code, do it here.
Comments or Future Work
Do we need to start another PR soon to address something you saw while working on this?
Workflow Checklist
Thank you!