Fix(project): Fix Failing tests #317

Temidayo32 · 2024-10-18T10:18:02Z

fix(project): Fix Failing tests

Because

Python 3.12 has updated standards for hosting web servers, which need to be reflected in the current implementation.
The current XPI format is outdated and needs to be updated for compatibility with modern Firefox.
The origin attribute is not defined on the AddOnInstallBlocked web element, and the name attribute only contains the origin host without the port.

This commit

Refactors the web server setup in webserver.py to use modern Python 3.12 practices for better compatibility and reliability.
Updates the web extension to a modern XPI format compatible with current Firefox versions.
Updates base.py to check the name attribute for the host value in place of origin.
Modifies test_notification_with_origin to assert only the host value when comparing with the webserver, as the port cannot be retrieved from the web element.

Fixes #308

…dards Because * Python 3.12 has updated standards for hosting web servers, which need to be reflected in the current implementation. This commit * Refactors the web server setup in webserver.py to use modern Python 3.12 practices for better compatibility and reliability. Fixes mozilla#308

Because * The current webextension.xpi is outdated and incompatible with modern web extensions in Firefox. This commit * Recreates the XPI to ensure compatibility with the latest Firefox standards. The Manifest file is updated to version 3. Fixes mozilla#308

b4handjr · 2024-10-18T14:31:52Z

Can you fix the name of this PR as well as check the output from the checks.

Because * The `origin` attribute is not defined on the `AddOnInstallBlocked` web element, and the `name` attribute only contains the origin host without the port. This commit * Updates `base.py` to check the `name` attribute for the host value in place of `origin`. * Modifies `test_notification_with_origin` to assert only the host value when comparing with the webserver, as the port cannot be retrieved from the web element. This might still require further revisions. Fixes mozilla#308

Temidayo32 · 2024-10-18T15:04:29Z

Can you fix the name of this PR as well as check the output from the checks.

Thanks @b4handjr. The tests are now passing here.

b4handjr · 2024-10-18T15:34:21Z

When you would like a review, please request one. Thanks.

Temidayo32 · 2024-10-18T16:08:52Z

@b4handjr I would like to request a review for my PR. Could you please review my PR? I would appreciate it.

b4handjr

This is a good start, but I am trying to run this locally and I can't. I am having some DNS resolution errors. Could you provide a fix for this?

Here is the error

E       selenium.common.exceptions.WebDriverException: Message: Reached error page: about:neterror?e=connectionFailure&u=http%3A//0.0.0.0%3A8000/&c=UTF-8&d=Firefox%20can%E2%80%99t%20establish%20a%20connection%20to%20the%20server%20at%200.0.0.0%3A8000.
E       Stacktrace:
E       RemoteError@chrome://remote/content/shared/RemoteError.sys.mjs:8:8
E       WebDriverError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:193:5
E       UnknownError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:832:5
E       checkReadyState@chrome://remote/content/marionette/navigate.sys.mjs:58:24
E       onNavigation@chrome://remote/content/marionette/navigate.sys.mjs:344:39
E       emit@resource://gre/modules/EventEmitter.sys.mjs:148:20
E       receiveMessage@chrome://remote/content/marionette/actors/MarionetteEventsParent.sys.mjs:33:25

.python-version

Makefile

tests/test_notifications.py

tests/webserver.py

Temidayo32 · 2024-10-18T18:13:56Z

This is a good start, but I am trying to run this locally and I can't. I am having some DNS resolution errors. Could you provide a fix for this?

Here is the error

E       selenium.common.exceptions.WebDriverException: Message: Reached error page: about:neterror?e=connectionFailure&u=http%3A//0.0.0.0%3A8000/&c=UTF-8&d=Firefox%20can%E2%80%99t%20establish%20a%20connection%20to%20the%20server%20at%200.0.0.0%3A8000.
E       Stacktrace:
E       RemoteError@chrome://remote/content/shared/RemoteError.sys.mjs:8:8
E       WebDriverError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:193:5
E       UnknownError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:832:5
E       checkReadyState@chrome://remote/content/marionette/navigate.sys.mjs:58:24
E       onNavigation@chrome://remote/content/marionette/navigate.sys.mjs:344:39
E       emit@resource://gre/modules/EventEmitter.sys.mjs:148:20
E       receiveMessage@chrome://remote/content/marionette/actors/MarionetteEventsParent.sys.mjs:33:25

Thanks for pointing it out. I will review promptly.

cashall-0 · 2024-10-19T05:52:36Z

is this issue fixed? seems like a blocker to successfully fix other issues.

Temidayo32 · 2024-10-19T12:11:49Z

This is a good start, but I am trying to run this locally and I can't. I am having some DNS resolution errors. Could you provide a fix for this?

Here is the error

E       selenium.common.exceptions.WebDriverException: Message: Reached error page: about:neterror?e=connectionFailure&u=http%3A//0.0.0.0%3A8000/&c=UTF-8&d=Firefox%20can%E2%80%99t%20establish%20a%20connection%20to%20the%20server%20at%200.0.0.0%3A8000.
E       Stacktrace:
E       RemoteError@chrome://remote/content/shared/RemoteError.sys.mjs:8:8
E       WebDriverError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:193:5
E       UnknownError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:832:5
E       checkReadyState@chrome://remote/content/marionette/navigate.sys.mjs:58:24
E       onNavigation@chrome://remote/content/marionette/navigate.sys.mjs:344:39
E       emit@resource://gre/modules/EventEmitter.sys.mjs:148:20
E       receiveMessage@chrome://remote/content/marionette/actors/MarionetteEventsParent.sys.mjs:33:25

Hi @b4handjr Here is my approach to fixing the problem.

The error shows there is a DNS resolution issue when trying to access http://0.0.0.0:8000/. And this is because 0.0.0.0 is being used as the host, which can cause issues in some environments. To solve this problem, I explicitly assigned 127.0.0.1 as the host in the conftest.py. Secondly, I also intoduced a new helper method get_free_port to dynamically assign any free port so as to avoid port conflicts too.

Because * DNS resolution errors can occur when running the web server locally, causing the server to fail in resolving hostnames. * Binding the web server to `127.0.0.1` resolves this issue by ensuring that the server listens only on the loopback address, preventing external DNS lookups. This commit * Modifies the `WebServer` class by adding a `get_free_port` method to dynamically assign ports. * The conftest.py now assigns `127.0.0.1` as a default port to the `WebServer` class. * Ensures that the server runs reliably in local development environments without requiring external DNS resolution. Fixes mozilla#308

b4handjr · 2024-10-19T14:12:02Z

is this issue fixed? seems like a blocker to successfully fix other issues.

You can try solving this as well, if you have an idea of how you think it should be done.

b4handjr · 2024-10-19T14:14:24Z

This is a good start, but I am trying to run this locally and I can't. I am having some DNS resolution errors. Could you provide a fix for this?
Here is the error
E       selenium.common.exceptions.WebDriverException: Message: Reached error page: about:neterror?e=connectionFailure&u=http%3A//0.0.0.0%3A8000/&c=UTF-8&d=Firefox%20can%E2%80%99t%20establish%20a%20connection%20to%20the%20server%20at%200.0.0.0%3A8000.
E       Stacktrace:
E       RemoteError@chrome://remote/content/shared/RemoteError.sys.mjs:8:8
E       WebDriverError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:193:5
E       UnknownError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:832:5
E       checkReadyState@chrome://remote/content/marionette/navigate.sys.mjs:58:24
E       onNavigation@chrome://remote/content/marionette/navigate.sys.mjs:344:39
E       emit@resource://gre/modules/EventEmitter.sys.mjs:148:20
E       receiveMessage@chrome://remote/content/marionette/actors/MarionetteEventsParent.sys.mjs:33:25
Hi @b4handjr Here is my approach to fixing the problem.

The error shows there is a DNS resolution issue when trying to access http://0.0.0.0:8000/. And this is because 0.0.0.0 is being used as the host, which can cause issues in some environments. To solve this problem, I explicitly assigned 127.0.0.1 as the host in the conftest.py. Secondly, I also intoduced a new helper method get_free_port to dynamically assign any free port so as to avoid port conflicts too.

Thanks, I will take a look. Also, your follow up commits don't need to be in the commit format for the PR, you can just leave a short message using git commit -m "message here" outlining what the specific commit did.

b4handjr · 2024-10-21T14:55:00Z

I noticed your tests are passing on release firefox. This is probably because your web extension is signed. Did you get your webextension signed? I see this ID in your manifest.json:

"browser_specific_settings": {
    "gecko": {
      "id": "{b18bb03d-7976-4449-9c85-73b3c93b56c8}"
    }
  }

Temidayo32 · 2024-10-21T15:34:15Z

I noticed your tests are passing on release firefox. This is probably because your web extension is signed. Did you get your webextension signed? I see this ID in your manifest.json:
"browser_specific_settings": {
    "gecko": {
      "id": "{b18bb03d-7976-4449-9c85-73b3c93b56c8}"
    }
  }

Yeah, it is signed. I tried the unsigned one, the AddOnInstallBlocked doesn't get triggered (the video below).

b4handjr · 2024-10-21T22:09:03Z

I noticed your tests are passing on release firefox. This is probably because your web extension is signed. Did you get your webextension signed? I see this ID in your manifest.json:
"browser_specific_settings": {
    "gecko": {
      "id": "{b18bb03d-7976-4449-9c85-73b3c93b56c8}"
    }
  }
Yeah, it is signed. I tried the unsigned one, the AddOnInstallBlocked doesn't get triggered (the video below).

Yeah that is what I thought would happen. I guess for full functionality this is the ideal approach but we should notify people of this limitation within Firefox as it is hard to uncover.

b4handjr

Looking good, I was able to test this locally and saw it getting a new port each test run.

tests/webserver.py

b4handjr · 2024-10-25T15:17:40Z

If no one else tries to fix this issue I will merge this in the next couple of days.

b4handjr

Thank you so much for this!

* refactor(project): Update webserver.py to align with Python 3.12 standards Because * Python 3.12 has updated standards for hosting web servers, which need to be reflected in the current implementation. This commit * Refactors the web server setup in webserver.py to use modern Python 3.12 practices for better compatibility and reliability. Fixes mozilla#308 * fix(project): Update XPI for modern Firefox compatibility Because * The current webextension.xpi is outdated and incompatible with modern web extensions in Firefox. This commit * Recreates the XPI to ensure compatibility with the latest Firefox standards. The Manifest file is updated to version 3. Fixes mozilla#308 * fix(base.py): Adjust origin handling for AddOnInstallBlocked Because * The `origin` attribute is not defined on the `AddOnInstallBlocked` web element, and the `name` attribute only contains the origin host without the port. This commit * Updates `base.py` to check the `name` attribute for the host value in place of `origin`. * Modifies `test_notification_with_origin` to assert only the host value when comparing with the webserver, as the port cannot be retrieved from the web element. This might still require further revisions. Fixes mozilla#308 * fix(project): Resolve DNS issues by binding to loopback address Because * DNS resolution errors can occur when running the web server locally, causing the server to fail in resolving hostnames. * Binding the web server to `127.0.0.1` resolves this issue by ensuring that the server listens only on the loopback address, preventing external DNS lookups. This commit * Modifies the `WebServer` class by adding a `get_free_port` method to dynamically assign ports. * The conftest.py now assigns `127.0.0.1` as a default port to the `WebServer` class. * Ensures that the server runs reliably in local development environments without requiring external DNS resolution. Fixes mozilla#308 * removed os module * eliminated custom translate_path

Temidayo32 force-pushed the bug branch from 5d01c74 to cf2d956 Compare October 18, 2024 10:29

Temidayo32 changed the title ~~refactor(project): Update webserver.py to align with Python 3.12 stan…~~ Fix Failing tests Oct 18, 2024

Temidayo32 added 2 commits October 18, 2024 15:11

Temidayo32 force-pushed the bug branch from cf2d956 to 4e14712 Compare October 18, 2024 14:21

Temidayo32 force-pushed the bug branch from 4e14712 to 6486669 Compare October 18, 2024 14:36

Temidayo32 force-pushed the bug branch from 6486669 to fd6daab Compare October 18, 2024 14:48

Temidayo32 changed the title ~~Fix Failing tests~~ Fix(project): Fix Failing tests Oct 18, 2024

b4handjr self-requested a review October 18, 2024 17:09

b4handjr requested changes Oct 18, 2024

View reviewed changes

.python-version Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

tests/test_notifications.py Outdated Show resolved Hide resolved

tests/webserver.py Outdated Show resolved Hide resolved

tests/webserver.py Show resolved Hide resolved

Temidayo32 force-pushed the bug branch from 9a4e78b to 03eb1c8 Compare October 19, 2024 13:50

b4handjr requested changes Oct 21, 2024

View reviewed changes

tests/webserver.py Outdated Show resolved Hide resolved

removed os module

337c5d8

b4handjr requested changes Oct 22, 2024

View reviewed changes

tests/webserver.py Outdated Show resolved Hide resolved

Temidayo32 force-pushed the bug branch from 018a55a to 69103fd Compare October 22, 2024 15:36

eliminated custom translate_path

63298bf

Temidayo32 force-pushed the bug branch from 69103fd to 63298bf Compare October 22, 2024 15:38

b4handjr self-requested a review October 22, 2024 16:50

b4handjr reviewed Oct 22, 2024

View reviewed changes

tests/webserver.py Show resolved Hide resolved

b4handjr approved these changes Oct 29, 2024

View reviewed changes

b4handjr merged commit 9979daf into mozilla:main Oct 29, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix(project): Fix Failing tests #317

Fix(project): Fix Failing tests #317

Temidayo32 commented Oct 18, 2024 •

edited

Loading

b4handjr commented Oct 18, 2024

Temidayo32 commented Oct 18, 2024

b4handjr commented Oct 18, 2024

Temidayo32 commented Oct 18, 2024 •

edited

Loading

b4handjr left a comment

Temidayo32 commented Oct 18, 2024

cashall-0 commented Oct 19, 2024

Temidayo32 commented Oct 19, 2024 •

edited

Loading

b4handjr commented Oct 19, 2024

b4handjr commented Oct 19, 2024

b4handjr commented Oct 21, 2024

Temidayo32 commented Oct 21, 2024 •

edited

Loading

b4handjr commented Oct 21, 2024 •

edited

Loading

b4handjr left a comment

b4handjr commented Oct 25, 2024

b4handjr left a comment

Fix(project): Fix Failing tests #317

Fix(project): Fix Failing tests #317

Conversation

Temidayo32 commented Oct 18, 2024 • edited Loading

b4handjr commented Oct 18, 2024

Temidayo32 commented Oct 18, 2024

b4handjr commented Oct 18, 2024

Temidayo32 commented Oct 18, 2024 • edited Loading

b4handjr left a comment

Choose a reason for hiding this comment

Temidayo32 commented Oct 18, 2024

cashall-0 commented Oct 19, 2024

Temidayo32 commented Oct 19, 2024 • edited Loading

b4handjr commented Oct 19, 2024

b4handjr commented Oct 19, 2024

b4handjr commented Oct 21, 2024

Temidayo32 commented Oct 21, 2024 • edited Loading

b4handjr commented Oct 21, 2024 • edited Loading

b4handjr left a comment

Choose a reason for hiding this comment

b4handjr commented Oct 25, 2024

b4handjr left a comment

Choose a reason for hiding this comment

Temidayo32 commented Oct 18, 2024 •

edited

Loading

Temidayo32 commented Oct 18, 2024 •

edited

Loading

Temidayo32 commented Oct 19, 2024 •

edited

Loading

Temidayo32 commented Oct 21, 2024 •

edited

Loading

b4handjr commented Oct 21, 2024 •

edited

Loading