[tests] Selenium test for Preview configuration with keyboard #868 #955

dee077 · 2024-12-24T20:21:50Z

Added a Selenium test case to replicate the process by creating a device, clicking its anchor tag, pressing ALT+P to open the preview, and pressing ESC to close it.

Fixes #868

Checklist

I have read the OpenWISP Contributing Guidelines.
I have manually tested the changes proposed in this pull request.
I have written new test cases for new code and/or updated existing tests for changes to existing code.

Reference to Existing Issue

Closes #868

Description of Changes:

Creates and saves a device configuration.
Navigate to the device detail page.
Simulates pressing Alt + P to open the preview configuration overlay.
Verifies that the overlay becomes visible.
Simulates pressing Escape to close the overlay.
Confirms that the overlay disappears.

Added a Selenium test case to replicate the process by creating a device, clicking its anchor tag, pressing ALT+P to open the preview, and pressing ESC to close it. Fixes openwisp#868

coveralls · 2024-12-24T20:31:33Z

coverage: 98.261%. remained the same
when pulling 28eee54 on dee077:issues/868-preview-keyboard-keys-selenium-test
into 98fd7b1 on openwisp:master.

…test

nemesifier

So far it's pretty good but I found some issues during testing.

I will push some changes shortly to address the shortcomings and show you how to better write these tests.

nemesifier · 2025-01-21T17:35:17Z

openwisp_controller/config/tests/test_selenium.py

+                lambda driver: driver.find_element(
+                    By.CSS_SELECTOR, '.djnjc-overlay'
+                ).value_of_css_property('display')
+                == 'none'


This assertion is ineffective and passes always, even if the closePreview JS function is broken on purpose.
Lesson: always break the tested code on purpose to verify the test catches the failure.

nemesifier · 2025-01-21T17:36:57Z

openwisp_controller/config/tests/test_selenium.py

+        try:
+            WebDriverWait(self.web_driver, 2).until(
+                lambda driver: driver.find_element(
+                    By.CSS_SELECTOR, '.djnjc-overlay'


Watch out! There's two .djnjc-overlay in the HTML page and in this code you were executing tests without ensuring you're running assertions on the right element!

Suggested change

By.CSS_SELECTOR, '.djnjc-overlay'

By.CSS_SELECTOR, '.djnjc-overlay:not(.loading)'

- Improved naming of test - Removed useless lines of code - Fail test explicitly with clear error message on Timeout exception instead of using assertTrue - Introduced subTest to separate phases for better readability

nemesifier

@dee077 please check my changes and read the commit description for an overview on the reasons of the changes: 937f2f2.

pandafy · 2025-01-22T09:29:06Z

openwisp_controller/config/tests/test_selenium.py

+        self._create_config(device=self._create_device(name='Test'))
+        self.login()
+        self.open(reverse('admin:config_device_changelist'))
+
+        # open first device in the list
+        self.web_driver.find_element(
+            by=By.CSS_SELECTOR, value='tbody tr:nth-child(1) th a'
+        ).click()


This can be shortened by directly opening the device page.

Suggested change

self._create_config(device=self._create_device(name='Test'))

self.login()

self.open(reverse('admin:config_device_changelist'))

# open first device in the list

self.web_driver.find_element(

by=By.CSS_SELECTOR, value='tbody tr:nth-child(1) th a'

).click()

device = self._create_config(device=self._create_device(name='Test')).device

self.login()

self.open(reverse('admin:config_device_change', args=[device.id]))

pandafy

We can further improve the test to make it robust and efficient.

pandafy · 2025-01-22T09:31:57Z

openwisp_controller/config/tests/test_selenium.py

+        try:
+            WebDriverWait(self.web_driver, 2).until(
+                EC.presence_of_element_located((By.CSS_SELECTOR, '#content-main'))
+            )
+        except TimeoutException:
+            self.fail('Device detail page did not load in time')


The open method already uses WebDriverWait. We can just wrap it in a try-except block.

https://github.com/openwisp/openwisp-utils/blob/1103df50164d7a6ac9b2cefa2f866f580b688111/openwisp_utils/test_selenium_mixins.py#L68-L70

pandafy · 2025-01-22T09:52:20Z

openwisp_controller/config/tests/test_selenium.py

+                WebDriverWait(self.web_driver, 2).until(
+                    lambda driver: driver.find_element(
+                        By.CSS_SELECTOR, '.djnjc-overlay:not(.loading)'
+                    ).value_of_css_property('display')
+                    == 'none'
+                )


Similarly, this can be changed to use EC.invisibility_of_element_located

Suggested change

WebDriverWait(self.web_driver, 2).until(

lambda driver: driver.find_element(

By.CSS_SELECTOR, '.djnjc-overlay:not(.loading)'

).value_of_css_property('display')

== 'none'

)

WebDriverWait(self.web_driver, 2).until(

EC.invisibility_of_element_located(

(By.CSS_SELECTOR, '.djnjc-overlay:not(.loading)')

)

)

pandafy · 2025-01-22T09:52:26Z

openwisp_controller/config/tests/test_selenium.py

+                WebDriverWait(self.web_driver, 2).until(
+                    lambda driver: driver.find_element(
+                        By.CSS_SELECTOR, '.djnjc-overlay:not(.loading)'
+                    ).value_of_css_property('display')
+                    != 'none'
+                )


Let's use EC.visibility_of_element_located

Suggested change

WebDriverWait(self.web_driver, 2).until(

lambda driver: driver.find_element(

By.CSS_SELECTOR, '.djnjc-overlay:not(.loading)'

).value_of_css_property('display')

!= 'none'

)

WebDriverWait(self.web_driver, 2).until(

EC.visibility_of_element_located(

(By.CSS_SELECTOR, '.djnjc-overlay:not(.loading)')

)

)

Refactored WebDriverWait usage to use built-in expected conditions. Fixes openwisp#868

nemesifier · 2025-01-22T18:25:52Z

openwisp_controller/config/tests/test_selenium.py

+            actions.key_down(Keys.ALT).send_keys('p').key_up(Keys.ALT).perform()
+            try:
+                WebDriverWait(self.web_driver, 2).until(
+                    EC.invisibility_of_element_located(


Shouldn't this be as follows?

Suggested change

EC.invisibility_of_element_located(

EC.visibility_of_element_located(

nemesifier · 2025-01-22T18:26:16Z

openwisp_controller/config/tests/test_selenium.py

+            actions = ActionChains(self.web_driver)
+            actions.key_down(Keys.ALT).send_keys('p').key_up(Keys.ALT).perform()
+            try:
+                WebDriverWait(self.web_driver, 2).until(


I guess we can change this to 1 as it's a fast operation.

Suggested change

WebDriverWait(self.web_driver, 2).until(

WebDriverWait(self.web_driver, 1).until(

nemesifier · 2025-01-22T18:26:24Z

openwisp_controller/config/tests/test_selenium.py

+            actions = ActionChains(self.web_driver)
+            actions.send_keys(Keys.ESCAPE).perform()
+            try:
+                WebDriverWait(self.web_driver, 2).until(


Suggested change

WebDriverWait(self.web_driver, 2).until(

WebDriverWait(self.web_driver, 1).until(

Manually removed ALT+P logic to ensure test fails with correct message. Fixes openwisp#868

nemesifier

LGTM 👍, thanks! 🙏

[tests] Selenium test for Preview configuration openwisp#868

6607106

Added a Selenium test case to replicate the process by creating a device, clicking its anchor tag, pressing ALT+P to open the preview, and pressing ESC to close it. Fixes openwisp#868

Merge branch 'master' into issues/868-preview-keyboard-keys-selenium-…

0b2c0e8

…test

nemesifier requested changes Jan 21, 2025

View reviewed changes

[chores] Addressed shortcomings

937f2f2

- Improved naming of test - Removed useless lines of code - Fail test explicitly with clear error message on Timeout exception instead of using assertTrue - Introduced subTest to separate phases for better readability

nemesifier reviewed Jan 21, 2025

View reviewed changes

pandafy reviewed Jan 22, 2025

View reviewed changes

pandafy requested changes Jan 22, 2025

View reviewed changes

[tests] Improve Selenium test efficiency openwisp#868

ade1905

Refactored WebDriverWait usage to use built-in expected conditions. Fixes openwisp#868

nemesifier requested changes Jan 22, 2025

View reviewed changes

[tests] Verify assertion check in Selenium test openwisp#868

28eee54

Manually removed ALT+P logic to ensure test fails with correct message. Fixes openwisp#868

nemesifier approved these changes Jan 22, 2025

View reviewed changes

nemesifier merged commit 211bebc into openwisp:master Jan 22, 2025
12 checks passed

	By.CSS_SELECTOR, '.djnjc-overlay'
	By.CSS_SELECTOR, '.djnjc-overlay:not(.loading)'

	EC.invisibility_of_element_located(
	EC.visibility_of_element_located(

	WebDriverWait(self.web_driver, 2).until(
	WebDriverWait(self.web_driver, 1).until(

Uh oh!

[tests] Selenium test for Preview configuration with keyboard #868 #955

[tests] Selenium test for Preview configuration with keyboard #868 #955

Uh oh!

Conversation

dee077 commented Dec 24, 2024

Checklist

Reference to Existing Issue

Description of Changes:

Uh oh!

coveralls commented Dec 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nemesifier left a comment

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!

nemesifier left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pandafy left a comment

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!

Choose a reason for hiding this comment

Uh oh!

nemesifier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Dec 24, 2024 •

edited

Loading