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.

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

[fix] Fixed flaky test: test_input_filters #436

Merged

Conversation

youhaveme9
Copy link
Contributor

@youhaveme9 youhaveme9 commented Feb 10, 2025

Checklist

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

Reference to Existing Issue

Closes #431

Description of Changes

Fixes flaky test in test_input_filer observed in CI.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Thanks @youhaveme9, please see my comments below.

@@ -125,7 +126,10 @@ def _get_menu_backdrop(self):
return self.web_driver.find_element(By.CSS_SELECTOR, '.menu-backdrop')

def _get_simple_input_filter(self):
return self.web_driver.find_element(By.CSS_SELECTOR, 'input[name=shelf]')
self.open(reverse('admin:test_project_shelf_changelist'))
return WebDriverWait(self.web_driver, 15).until(
Copy link
Member

Choose a reason for hiding this comment

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

are we waiting 15 seconds? Why so much? Totally unnecessary!
2 seconds should be a lot more than enough..

@@ -125,7 +126,10 @@ def _get_menu_backdrop(self):
return self.web_driver.find_element(By.CSS_SELECTOR, '.menu-backdrop')

def _get_simple_input_filter(self):
return self.web_driver.find_element(By.CSS_SELECTOR, 'input[name=shelf]')
self.open(reverse('admin:test_project_shelf_changelist'))
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 method used only once? If so, please move this code to the test method, this separation isn't necessary and makes the code less readable with no benefits.

@@ -590,7 +590,9 @@ def test_input_filters(self):
input_field.send_keys('Horror')
self._get_filter_button().click()
# Horror shelf is present
self.web_driver.find_element(By.XPATH, horror_result_xpath)
WebDriverWait(self.web_driver, 10).until(
Copy link
Member

Choose a reason for hiding this comment

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

is that 10 seconds again?

"Bitstream Vera Sans", Verdana, Arial, sans-serif;
--font-family-primary:
"Roboto", "Lucida Grande", "DejaVu Sans", "Bitstream Vera Sans", Verdana,
Arial, sans-serif;
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 change necessary? Who's causing this? Is it prettier or what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, idk why it was skipped before

@coveralls
Copy link

coveralls commented Feb 18, 2025

Coverage Status

coverage: 96.18%. remained the same
when pulling 22ce148 on youhaveme9:issues/431-falky-test-input-filter
into 1d648f5 on openwisp:master.

@youhaveme9
Copy link
Contributor Author

hey @nemesifier I have done the required changes but for that I have to make some workaround which I think can be improved in test_selenium.py line 530
It was done because running the test in loop 1 in 10 times the current url redirects to /admin from /admin/test_project/shelf/ so I'm again opening the url /admin/test_project/shelf/ when its redirected to /admin

Please suggest or give a hint for improving this PR
Thanks

@nemesifier nemesifier added the bug label Feb 18, 2025
@nemesifier nemesifier merged commit 8ffb088 into openwisp:master Feb 18, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

[bug] Flaky test: test_input_filters
3 participants