posix4e / puppet

MIT License
8 stars 8 forks source link

Cleanup e2e tests #34

Closed vkolgi closed 1 year ago

vkolgi commented 1 year ago
  1. For easier local testing, moved SAUCE secrets to .env file
  2. Got rid of unnecessary sleeps
  3. Changes required by the past review by GPT bot
  4. Explicit timeout is added in case of waiting for elements to appear.
github-actions[bot] commented 1 year ago

LOGAF Level 2 - /home/runner/work/puppet/puppet/e2e_tests/test_sauce_labs.py

1. Avoid hardcoding values:

Instead of hardcoding values like "http://127.0.0.1:4723", "https://ondemand.us-west-1.saucelabs.com:443/wd/hub", "com.ttt246.puppet", ".ChatterAct", etc., consider using environment variables or a configuration file.

2. Avoid using time.sleep():

The use of time.sleep() is generally discouraged as it can lead to inefficient waiting. Consider using explicit waits or other synchronization methods.

3. Exception handling:

In the create_android_driver function, there is no exception handling for the case when the connection to the webdriver fails. Consider adding a try-except block to handle potential exceptions.

4. Code repetition:

There is some repetition in the save_server_settings and enable_privacy_settings functions. Consider creating a helper function to reduce the repetition.

Example:

def find_element_and_send_keys(driver, by_selector, selector_value, keys):
    element = driver.find_element(by=by_selector, value=selector_value)
    element.send_keys(keys)
    return element

Then you can replace the repeated code with:

find_element_and_send_keys(driver, AppiumBy.ID, f"{APP_PACKAGE_NAME}:id/uuidEditText", TEST_UUID)

LOGAF Level 3 - /home/runner/work/puppet/puppet/e2e_tests/utils.py

1. Exception handling:

In the wait_for_element function, a bare except clause is used. This is generally discouraged as it can catch unexpected exceptions. Consider specifying the exception type.

2. Return statement in finally block:

The return statement in the finally block can lead to unexpected behavior as it will always execute, potentially swallowing exceptions. Consider moving the return statement out of the finally block.

Example:

try:
    element = WebDriverWait(driver, timeout).until(
        EC.presence_of_element_located((by_selector, selector_value))
    )
except TimeoutException:
    element = None

return element

🔧💤🔁


Powered by Code Review GPT

vkolgi commented 1 year ago

This can be closed now since I have merged the changes from this branch to #39 since they had to go together, otherwise E2E tests would fail. The necessary changes raised by bot is also incorporated in the PR #39.