Skip to content

[tests] Add basic Selenium tests for Dependabot PRs (#537)#584

Closed
aoutifrak wants to merge 0 commit intoopenwisp:masterfrom
aoutifrak:master
Closed

[tests] Add basic Selenium tests for Dependabot PRs (#537)#584
aoutifrak wants to merge 0 commit intoopenwisp:masterfrom
aoutifrak:master

Conversation

@aoutifrak
Copy link
Contributor

@aoutifrak aoutifrak commented Mar 21, 2025

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.
  • I have updated the documentation.

Reference to Existing Issue

Closes #537.

Please open a new issue if there isn't an existing issue yet.

Description of Changes

Please describe these changes.

Screenshot

Please include any relevant screenshots.

Copy link
Contributor Author

@aoutifrak aoutifrak left a comment

Choose a reason for hiding this comment

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

Fixed the formatting issues to comply with style guidelines.

@aoutifrak aoutifrak requested a review from nemesifier March 22, 2025 17:45
@codesankalp codesankalp self-requested a review March 22, 2025 17:52
Copy link
Member

@codesankalp codesankalp left a comment

Choose a reason for hiding this comment

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

Hi @aoutifrak, thanks for your contribution!
Please take a look at my comments below. For the Selenium tests, you can refer to other repositories like openwisp-controller to avoid duplication and follow best practices for implementing this feature.

Copy link
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

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

@aoutifrak I have left suggestions to make the tests consistent with other OpenWISP modules.

@github-project-automation github-project-automation bot moved this from Needs review to In progress in OpenWISP Contributor's Board Mar 24, 2025
@aoutifrak aoutifrak requested a review from pandafy March 24, 2025 23:17
@aoutifrak aoutifrak requested a review from codesankalp March 26, 2025 16:57
Copy link
Contributor Author

@aoutifrak aoutifrak left a comment

Choose a reason for hiding this comment

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

Updated test cases for batch user creation, CSV import, and user generation.
Replaced double quotes with single quotes for string literals to maintain code consistency.
Added detailed comments to explain each part of the test case logic for better understanding.
Ensured alignment with OpenWISP conventions, including the use of self.open() and self.find_element().
These changes enhance the clarity and maintainability of the tests.

Copy link
Contributor Author

@aoutifrak aoutifrak left a comment

Choose a reason for hiding this comment

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

  • Added SELENIUM_HEADLESS=1 to the workflow environment variables.
  • Ensures Selenium tests run in headless mode in CI for consistency.
  • Helps prevent UI-related flakiness in automated tests.

This change improves reliability and aligns the workflow with best practices for Selenium testing in CI environments.

Copy link
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

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

Thanks for contributing! I have left some suggestions to improve the tests.

@@ -0,0 +1,3 @@
csv-user1,password1,csvuser1@example.com,CSV1,User1
csv-user2,password2,csvuser2@example.com,CSV2,User2
csv-user3,password3,csvuser3@example.com,CSV3,User3 No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Add a blank line at end of the file. You can configure your code editor to always do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

endline are exsiste just didn't show here

Comment on lines +19 to +21
self.open(reverse('admin:openwisp_users_organization_add'))
self.find_element(By.ID, "id_name").send_keys("Test Batch")
self.find_element(By.CSS_SELECTOR, "input[type=submit]").click()
Copy link
Member

Choose a reason for hiding this comment

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

You can use self._get_org() helper method to create an organization here.


self.find_element(By.ID, "id_name").send_keys("Test Batch")
prefix_field = self.find_element(By.ID, "id_prefix")
prefix_field.clear()
Copy link
Member

Choose a reason for hiding this comment

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

Why are we clearing the field before entering data? Wouldn't the field be already empty?

self.find_element(By.ID, "id_name").send_keys("Test Batch")
self.find_element(By.CSS_SELECTOR, "input[type=submit]").click()

self.open("/admin/openwisp_radius/radiusbatch/add/")
Copy link
Member

Choose a reason for hiding this comment

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

Let's maintain consistent quoting style.

Also, use django.urls.reverse to get the path of the page. This applies to all the occurrences.

class BasicTest(
SeleniumTestMixin, FileMixin, StaticLiveServerTestCase, TestOrganizationMixin
):
browser = 'chrome'
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason for using chrome browser for this test? If not, we can remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is no reason thanks

Comment on lines +117 to +127
# Verify that users from the CSV file were created
self.find_element(By.CLASS_NAME, 'field-name').click()
self.open(reverse('admin:openwisp_users_user_changelist'))
user1_link = self.wait_for_visibility(
By.XPATH, "//a[contains(text(), 'user1')]", 10
)
self.assertIsNotNone(user1_link)

# Ensure another user from the CSV is also created
user2_link = self.find_element(By.XPATH, "//a[contains(text(), 'user2')]")
self.assertIsNotNone(user2_link)
Copy link
Member

Choose a reason for hiding this comment

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

Verify that user were created in the database using

self.assertEqual(User.objects.filter(username='<user1>).exists(), True)
self.assertEqual(User.objects.filter(username='<user2>).exists(), True)

Comment on lines +137 to +140
# Navigate to organization creation page
self.open(reverse('admin:openwisp_users_organization_add'))
self.find_element(By.ID, 'id_name', 10).send_keys('Test Batch')
self.find_element(By.CSS_SELECTOR, 'input[type=submit]', 10).click()
Copy link
Member

Choose a reason for hiding this comment

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

Use self._get_org() to create organization object.

Comment on lines +173 to +184
# Verify that users with hashed passwords are created
self.find_element(By.CLASS_NAME, 'field-name').click()
self.open(reverse('admin:openwisp_users_user_changelist'))
hash_user1_link = self.wait_for_visibility(
By.XPATH, "//a[contains(text(), 'hash_user1')]", 10
)
self.assertIsNotNone(hash_user1_link)

hash_user2_link = self.find_element(
By.XPATH, "//a[contains(text(), 'hash_user2')]", 10
)
self.assertIsNotNone(hash_user2_link)
Copy link
Member

Choose a reason for hiding this comment

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

You can verify that the users are created using

self.assertEqual(User.objects.filter(username='<>').exists(), Trie)

We should also verify that the password works for the user. Otherwise, this test is similar to one above it.

Comment on lines +191 to +194
# Create an organization for the prefix user generation
self.open(reverse('admin:openwisp_users_organization_add'))
self.find_element(By.ID, 'id_name', 10).send_keys('Test Batch')
self.find_element(By.CSS_SELECTOR, 'input[type=submit]', 10).click()
Copy link
Member

Choose a reason for hiding this comment

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

Use self._get_org() to create organization object.

assert contains_ten, "No element contains the number 10"

# Test case for user generation using CSV file upload
def test_csv_user_generation(self):
Copy link
Member

Choose a reason for hiding this comment

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

Is this a duplicate test?

@aoutifrak aoutifrak closed this Apr 2, 2025
@github-project-automation github-project-automation bot moved this from In progress to Done in OpenWISP Contributor's Board Apr 2, 2025
@nemesifier
Copy link
Member

@aoutifrak why did you close this pull request?

@nemesifier
Copy link
Member

Reopening.

Copy link
Contributor Author

@aoutifrak aoutifrak left a comment

Choose a reason for hiding this comment

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

failed to reopen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[tests] Add basic selenium tests to give us confidence on dependabot PRs

4 participants