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

feat: Add Windows Calculator Integration and Tests #97

Merged
merged 28 commits into from
Feb 12, 2025

Conversation

theijhay
Copy link
Collaborator

@theijhay theijhay commented Feb 1, 2025

Description:
This PR introduces the following changes:

  1. Windows Calculator Integration:

    • Configured OpenAppTransaction to launch the Windows Calculator using WinAppDriver.
    • Configured CloseAppTransaction to close the Windows Calculator.
  2. Calculator Transactions:

    • Created CalculatorTransactions class to perform calculator operations, click number buttons, and extract the result.
    • Added logging and error handling to the methods.
  3. Tests:

    • Created TestCalculator class to test the Windows Calculator integration.
    • The tests include performing an addition operation and validating the result.

Changes:

  • setup.py: Added OpenAppTransaction and CloseAppTransaction classes.
  • calculator.py: Added CalculatorTransactions class with logging and error handling.
  • test_calculator.py: Added TestCalculator class with addition test.

@theijhay theijhay requested a review from douglasdcm February 1, 2025 23:50
…eadability

Refactor CalculatorTransactions to add logging and error handling

Update test_calculator.py to pass driver object to OpenAppTransaction and CloseAppTransaction
Copy link
Owner

@douglasdcm douglasdcm left a comment

Choose a reason for hiding this comment

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

Thanks the PR. Fundamental issues are present in the implementation. Check the examples in the tutorial to get more details, please.

theijhay and others added 2 commits February 2, 2025 09:22
* Add pre-commit configuration

* test

* Add black workflow

* Formatting the entire code

* Set line length
@theijhay theijhay requested a review from douglasdcm February 2, 2025 09:18
Copy link
Owner

@douglasdcm douglasdcm left a comment

Choose a reason for hiding this comment

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

Checks failing. I will wait them be fixed before perform a new review

douglasdcm and others added 3 commits February 3, 2025 23:56
…ouglasdcm#92)

* `Implement AENA flight data scraper using Selenium and BeautifulSoup`

**Description:**
- Added web scraping functionality to collect flight arrival data from AENA's website.
- Utilized Selenium WebDriver with headless Chrome for automation.
- Implemented BeautifulSoup for parsing flight details.
- Introduced transaction-based execution using `guara.transaction`.
- Stored flight data in JSON format while filtering outdated entries.
- Handled potential exceptions like `TimeoutException` and `NoSuchElementException`.
- Updated script status tracking for better monitoring.

* Fix linter issues

* Running pre-commit
@theijhay
Copy link
Collaborator Author

theijhay commented Feb 4, 2025

Hi @douglasdcm

I’ve been working on the WinAppDriver integration for the Calculator app tests. However, I encountered an issue: WinAppDriver is a Windows-only tool and cannot run on Linux (Ubuntu).

Here’s the situation and potential solutions:

Limitation:

WinAppDriver requires a Windows environment to function.

The tests fail on Linux because the WinAppDriver server cannot be started.

Proposed Solutions:

Option 1: Run the tests on a Windows machine or CI/CD pipeline (e.g., GitHub Actions with a Windows runner).

Option 2: Use a cross-platform alternative like Appium for desktop apps (if applicable).

Option 3: Skip WinAppDriver tests on non-Windows platforms by adding conditional logic.

Let me know which approach you prefer.

If we proceed with Option 3, I can update the PR to skip WinAppDriver tests on Linux.

Looking forward to your feedback!

@douglasdcm
Copy link
Owner

Hi @douglasdcm

I’ve been working on the WinAppDriver integration for the Calculator app tests. However, I encountered an issue: WinAppDriver is a Windows-only tool and cannot run on Linux (Ubuntu).

Here’s the situation and potential solutions:

Limitation:

WinAppDriver requires a Windows environment to function.

The tests fail on Linux because the WinAppDriver server cannot be started.

Proposed Solutions:

Option 1: Run the tests on a Windows machine or CI/CD pipeline (e.g., GitHub Actions with a Windows runner).

Option 2: Use a cross-platform alternative like Appium for desktop apps (if applicable).

Option 3: Skip WinAppDriver tests on non-Windows platforms by adding conditional logic.

Let me know which approach you prefer.

If we proceed with Option 3, I can update the PR to skip WinAppDriver tests on Linux.

Looking forward to your feedback!

Thanks @theijhay we can proceed the same way we did for tests that run on Linux calculator. They are skip. We just need to guarantee it runs locally with Python 3.11. after that we can skip the test.. Maybe use lazy imports too if the win app driver is required while running the tests

@theijhay theijhay requested a review from douglasdcm February 8, 2025 04:21
Copy link
Owner

@douglasdcm douglasdcm 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 the PR. I gave some comments. Add a README file explaining the purpose of the example and the tools involved. Check other examples, please

@@ -233,14 +232,21 @@ def get_aena_data():
script_status = read_json_file("script_status.json", {"airports": {}, "status": None})

# Open the AENA page
app.at(OpenAenaPage)
Copy link
Owner

Choose a reason for hiding this comment

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

It is not correct. Consider the changes from the original file. In this crawler I use the new method 'when' to improve readability

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not touch this code sir

Copy link
Owner

Choose a reason for hiding this comment

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

I know. I changed at by when. I may be changed back while fixing conflicts . Compara the code against the main branch, please

logging.error(f"Error extracting result: {e}")
raise

def validate_result(self, expected: int):
Copy link
Owner

Choose a reason for hiding this comment

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

This method can be removed

from examples.windows_desktop.winappdriver import calculator

self.app.at(calculator.SumNumbers, num1=5, num2=3).asserts(
calculator.SumNumbers().validate_result, 8
Copy link
Owner

Choose a reason for hiding this comment

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

Not correct. Use the syntax app.at in high-level design.

…code

Refactor get_aena_data() function in main.py

Update black language version to python3.11 in .pre-commit-config.yaml

Remove complex setup in CI environment for Windows Calculator tests

Update test_calculator.py to use it.IAssertion for checking if value is shown in the calculator

Update test_calculator.py to use lazy import to avoid breaking the pipeline

Update test_calculator.py to use parametrize decorator for addition test cases
@theijhay theijhay requested a review from douglasdcm February 8, 2025 21:16
@theijhay
Copy link
Collaborator Author

theijhay commented Feb 9, 2025

Thanks for the PR. I gave some comments. Add a README file explaining the purpose of the example and the tools involved. Check other examples, please

Added

@@ -233,14 +232,21 @@ def get_aena_data():
script_status = read_json_file("script_status.json", {"airports": {}, "status": None})

# Open the AENA page
app.at(OpenAenaPage)
Copy link
Owner

Choose a reason for hiding this comment

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

I know. I changed at by when. I may be changed back while fixing conflicts . Compara the code against the main branch, please

Copy link
Owner

@douglasdcm douglasdcm left a comment

Choose a reason for hiding this comment

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

Thanks, but there are some comments not fixed yet

self._driver = webdriver.Remote(
command_executor="http://localhost:4723/wd/hub", options=options
)
self._application = Application(self._driver)
Copy link
Owner

Choose a reason for hiding this comment

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

Not correct. The application is the involker of the automation. It call the transactions not the opposite. Check other examples, please

Refactor setup.py: Remove unused import in winappdriver
Refactor test_calculator.py: Remove unused import and skip complex setup
@theijhay theijhay requested a review from douglasdcm February 11, 2025 22:10
Copy link
Owner

@douglasdcm douglasdcm left a comment

Choose a reason for hiding this comment

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

The example is OK. It is just necessary to revert the changes in the crawler file. It may be caused by conflicts solving

@douglasdcm douglasdcm merged commit b8b19be into douglasdcm:main Feb 12, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants