Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[fix] Fixed monitoring charts not loading from device admin #329 #349

Merged
merged 5 commits into from
Aug 28, 2024
Merged

[fix] Fixed monitoring charts not loading from device admin #329 #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
42c9290
[fix] Fixed monitoring charts not loading from device admin #329
pandafy Aug 27, 2024
60cea8b
[tests] Fixed tests
pandafy Aug 27, 2024
1f439e6
[chores] Updated openwisp-users commit hash
pandafy Aug 28, 2024
140a55a
[tests] Improved test
pandafy Aug 28, 2024
2dfd08b
[chores] Update images/openwisp_base/Dockerfile
nemesifier Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions images/common/openwisp/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'allauth.account.middleware.AccountMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
'django.middleware.clickjacking.XFrameOptionsMiddleware',
]
Expand Down
14 changes: 7 additions & 7 deletions images/openwisp_base/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@ ENV PYTHONPATH=/home/openwisp/.local/lib/python3.10/site-packages

RUN pip install --no-cache-dir --user --upgrade pip~=24.1.2 setuptools~=70.3.0 wheel~=0.43.0
# TODO: Remove when next version of openwisp-monitoring is released
ARG OPENWISP_MONITORING_SOURCE=https://github.com/openwisp/openwisp-monitoring/tarball/fb2814b6dd8213ddb45a8497cf41dfb10eee04a2
ARG OPENWISP_MONITORING_SOURCE=https://github.com/openwisp/openwisp-monitoring/tarball/b7e14204c84858f874c04e989dc7bba639712cf1
# hadolint ignore=DL3013
RUN pip install --no-cache-dir --user --upgrade ${OPENWISP_MONITORING_SOURCE}
ARG OPENWISP_FIRMWARE_SOURCE=https://github.com/openwisp/openwisp-firmware-upgrader/tarball/178055284b39c4f6d0b166e817a275efb9fc0dc0
ARG OPENWISP_FIRMWARE_SOURCE=https://github.com/openwisp/openwisp-firmware-upgrader/tarball/7434ad9b478a85d64697840c5d3a5c26ab3fd45e
# hadolint ignore=DL3013
RUN pip install --no-cache-dir --user --upgrade ${OPENWISP_FIRMWARE_SOURCE}
ARG OPENWISP_TOPOLOGY_SOURCE=https://github.com/openwisp/openwisp-network-topology/tarball/7ea037679a185be81a453fd3d5f0b820f5294da7
ARG OPENWISP_TOPOLOGY_SOURCE=https://github.com/openwisp/openwisp-network-topology/tarball/88b4f01466ce9495e8b0c5a962cefe9af606a156
# hadolint ignore=DL3013
RUN pip install --no-cache-dir --user --upgrade ${OPENWISP_TOPOLOGY_SOURCE}
ARG OPENWISP_RADIUS_SOURCE=https://github.com/openwisp/openwisp-radius/tarball/fd8a2300d65d6191d10c62b9ea4e1f56de2bcfe4
ARG OPENWISP_RADIUS_SOURCE=https://github.com/openwisp/openwisp-radius/tarball/489ac5386be68636a806496c90ddb094772e9270
# hadolint ignore=DL3013
RUN pip install --no-cache-dir --user --upgrade ${OPENWISP_RADIUS_SOURCE}

Expand All @@ -49,7 +49,7 @@ RUN if [ "$OPENWISP_IPAM_SOURCE" != "default" ] ; then \
pip install --no-cache-dir --user --upgrade ${OPENWISP_IPAM_SOURCE}; \
fi
# TODO: Remove when next version of openwisp-controller is released
ARG OPENWISP_CONTROLLER_SOURCE=https://github.com/openwisp/openwisp-controller/tarball/1a24f57493b1343f4e94409be3c24bdb5b952db1
ARG OPENWISP_CONTROLLER_SOURCE=https://github.com/openwisp/openwisp-controller/tarball/8acc0155a250f4a588f1ab5c70837cb0eee8731f
# hadolint ignore=DL3013
RUN if [ "$OPENWISP_CONTROLLER_SOURCE" != "default" ] ; then \
pip install --no-cache-dir --user --upgrade ${OPENWISP_CONTROLLER_SOURCE}; \
Expand All @@ -59,13 +59,13 @@ ARG OPENWISP_NOTIFICATION_SOURCE=default
RUN if [ "$OPENWISP_NOTIFICATION_SOURCE" != "default" ] ; then \
pip install --no-cache-dir --user --upgrade ${OPENWISP_NOTIFICATION_SOURCE}; \
fi
ARG OPENWISP_USERS_SOURCE=https://github.com/openwisp/openwisp-users/tarball/1130d5a4cbde0a09f57df638bba45bd53ea27192
ARG OPENWISP_USERS_SOURCE=https://github.com/openwisp/openwisp-users/tarball/dcd828d46401ccba47cfa6e3141b8eef59ab9de6
# hadolint ignore=DL3013
RUN if [ "$OPENWISP_USERS_SOURCE" != "default" ] ; then \
pip install --no-cache-dir --user --upgrade --force-reinstall ${OPENWISP_USERS_SOURCE}; \
fi
# TODO: Remove when next version of openwisp-utils is released
ARG OPENWISP_UTILS_SOURCE="openwisp-utils[celery,rest] @ https://github.com/openwisp/openwisp-utils/tarball/a93483530352e4919901959449476988fef55b33"
ARG OPENWISP_UTILS_SOURCE="openwisp-utils[celery,rest] @ https://github.com/openwisp/openwisp-utils/tarball/14578495c9be77dbb963ef97a7c06387cc12df83"
# hadolint ignore=DL3013
RUN if [ "$OPENWISP_UTILS_SOURCE" != "default" ] ; then \
pip install --no-cache-dir --user --upgrade --force-reinstall "${OPENWISP_UTILS_SOURCE}"; \
Expand Down
2 changes: 2 additions & 0 deletions images/openwisp_dashboard/module_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@
OPENWISP_MONITORING_API_URLCONF = 'openwisp_monitoring.urls'
OPENWISP_RADIUS_API_URLCONF = 'openwisp_radius.urls'
OPENWISP_NETWORK_TOPOLOGY_API_BASEURL = API_BASEURL
OPENWISP_NOTIFICATIONS_HOST = API_BASEURL
OPENWISP_CONTROLLER_API_HOST = API_BASEURL
OPENWISP_MONITORING_API_BASEURL = API_BASEURL
OPENWISP_FIRMWARE_API_BASEURL = API_BASEURL
OPENWISP_RADIUS_API_BASEURL = API_BASEURL
16 changes: 16 additions & 0 deletions tests/data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Initial data for running the tests

from openwisp_controller.config.models import Config, Device
from openwisp_radius.models import (
OrganizationRadiusSettings,
RadiusGroup,
Expand Down Expand Up @@ -58,13 +59,28 @@ def create_default_radiusUser(admin, radGroup):
return radiusUser


def create_device(organization):
if Device.objects.filter(name='test-device').exists():
return Device.objects.get(name='test-device')
device = Device(
name='test-device', mac_address='11:22:33:44:55:66', organization=organization
)
device.full_clean()
device.save()
config = Config(device=device, backend='netjsonconfig.OpenWrt')
config.full_clean()
config.save()
return device


def setup():
defOrg = get_organization()
admin = get_admin()
radGroup = get_default_radius_group()
create_default_organizationUser(defOrg, admin)
create_default_radiusUser(admin, radGroup)
set_default_radius_token(defOrg)
create_device(defOrg)


if __name__ == '__main__':
Expand Down
30 changes: 22 additions & 8 deletions tests/runtests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
from urllib import request

from selenium import webdriver
from selenium.common.exceptions import NoSuchElementException
from selenium.common.exceptions import NoSuchElementException, TimeoutException
from selenium.webdriver.chrome.options import Options as ChromiumOptions
from selenium.webdriver.common.by import By
from selenium.webdriver.common.desired_capabilities import DesiredCapabilities
from selenium.webdriver.support import expected_conditions as EC
from selenium.webdriver.support.ui import WebDriverWait
from utils import TestUtilities


Expand Down Expand Up @@ -121,7 +123,6 @@ def setUpClass(cls):
# Create base drivers (Chromium)
if cls.config['driver'] == 'chromium':
options = ChromiumOptions()
options.add_argument('--headless')
options.add_argument('--ignore-certificate-errors')
if cls.config['headless']:
options.add_argument('--headless')
Expand Down Expand Up @@ -190,6 +191,23 @@ def test_admin_login(self):
)
self.fail(message)

def test_device_monitoring_charts(self):
self.login()
self._wait_for_element()
self.get_resource('test-device', '/admin/config/device/')
self._wait_for_element()
self.base_driver.find_element(By.CSS_SELECTOR, 'ul.tabs li.charts').click()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried adding only this test on the latest master and it fails here, I was expecting it to fail in the try/except block below, but that's not the case. Is this intended or not?

Failure output:

ERROR: test_device_monitoring_charts (__main__.TestServices)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/runtests.py", line 198, in test_device_monitoring_charts
    self.base_driver.find_element(By.CSS_SELECTOR, 'ul.tabs li.charts').click()
  File "/lib/python3.8/site-packages/selenium/webdriver/remote/webdriver.py", line 748, in find_element
    return self.execute(Command.FIND_ELEMENT, {"using": by, "value": value})["value"]
  File "/lib/python3.8/site-packages/selenium/webdriver/remote/webdriver.py", line 354, in execute
    self.error_handler.check_response(response)
  File "/lib/python3.8/site-packages/selenium/webdriver/remote/errorhandler.py", line 229, in check_response
    raise exception_class(message, screen, stacktrace)
selenium.common.exceptions.NoSuchElementException: Message: no such element: Unable to locate element: {"method":"css selector","selector":"ul.tabs li.charts"}
  (Session info: chrome-headless-shell=127.0.6533.99); For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to replicate this error locally. Did you also copy the changes in data.py and utis.py?

Anyway, I have improved the test to make it more robust.

try:
WebDriverWait(self.base_driver, 3).until(EC.alert_is_present())
except TimeoutException:
# No alert means that the request to fetch
# monitoring charts was successful.
pass
else:
# When the request to fetch monitoring charts fails,
# an error is shown.
self.fail('An alert was found on the device chart page.')

def test_create_prefix_users(self):
self.login()
prefix_objname = 'automated-prefix-test-01'
Expand Down Expand Up @@ -265,10 +283,6 @@ def test_console_errors(self):
# url_list tests
for url in url_list:
self.base_driver.get(f"{self.config['app_url']}{url}")
# console_error_check method should be called twice
# to avoid the "beforeunload' chrome issue
# https://stackoverflow.com/questions/10680544/beforeunload-chrome-issue
self.console_error_check()
self.assertEqual([], self.console_error_check())
self.assertIn('OpenWISP', self.base_driver.title)
# change_form_list tests
Expand Down Expand Up @@ -311,10 +325,10 @@ def test_forgot_password(self):
"""Test forgot password to ensure that postfix is working properly."""
self.base_driver.get(f"{self.config['app_url']}/accounts/password/reset/")
self.base_driver.find_element(By.NAME, 'email').send_keys('[email protected]')
self.base_driver.find_element(By.XPATH, '//input[@type="submit"]').click()
self.base_driver.find_element(By.XPATH, '//button[@type="submit"]').click()
self._wait_for_element()
self.assertIn(
'We have sent you an e-mail. If you have not received '
'We have sent you an email. If you have not received '
'it please check your spam folder. Otherwise contact us '
'if you do not receive it in a few minutes.',
self.base_driver.page_source,
Expand Down
8 changes: 8 additions & 0 deletions tests/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,14 @@ def console_error_check(self, driver=None):
logs = driver.get_log('browser')
for logentry in logs:
if logentry['level'] == 'SEVERE':
# Ignore error generated due to "leaflet" issue
# https://github.com/makinacorpus/django-leaflet/pull/380
if 'leaflet' in logentry['message']:
continue
# Ignore error generated due to "beforeunload" chrome issue
# https://stackoverflow.com/questions/10680544/beforeunload-chrome-issue
if 'beforeunload' in logentry['message']:
continue
console_logs.append(logentry['message'])
return console_logs

Expand Down