Conversation
- Introduced new test files for better organization: - `test_simulate_batch.py`: Integration tests for batch simulation functionality. - `test_simulation.py`: Unit tests for single simulation execution and datastream parameters. - `test_gui_automation.py`: GUI automation tests for external application interaction. - `test_interactive.py`: Interactive tests requiring user input for simulation execution. - Removed deprecated test files: - `test_basic.py`: Redirects to new organized test files. - `test_cache_behavior.py`: Removed due to restructuring. - `test_entrypoint.py`: Removed as part of cleanup. - `test_parser.py`: Removed due to restructuring. - `test_parser_and_core.py`: Removed as part of cleanup. - `test_plecs_integration_simple.py`: Removed due to restructuring. - `test_smoke.py`: Removed as part of cleanup. - `test_automated.py`: Removed due to restructuring. - Enhanced existing tests with additional assertions and error handling. - Updated import paths and configurations to align with new test structure.
There was a problem hiding this comment.
Pull Request Overview
This is a major refactoring to remove the pywinauto dependency and clean up GUI automation functionality. The changes focus on simplifying the codebase by removing Windows-specific GUI automation while maintaining the core XML-RPC PLECS integration functionality.
- Remove pywinauto dependency from package requirements and installer scripts
- Refactor GUI automation code to raise NotImplementedError instead of using pywinauto
- Reorganize test files into proper directory structure with integration and e2e subdirectories
Reviewed Changes
Copilot reviewed 34 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/installers/windows_installer.ps1 | Remove pywinauto from package installation list |
| pyproject.toml | Remove pywinauto from full feature set dependencies |
| pyplecs/pyplecs.py | Remove pywinauto import and replace GUI automation with NotImplementedError |
| pyplecs/api/init.py | Modernize FastAPI with lifespan context manager |
| pyplecs/orchestration/init.py | Add imports for SimulationPlan and SimulationViewer |
| Multiple test files | Remove old test files and reorganize into integration/e2e structure |
| cli_demo_nomocks.py | Update imports to use orchestration module |
| Documentation files | Update references to remove pywinauto mentions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| try { | ||
| & $venvPython -m pip install --upgrade pip | ||
| & $venvPython -m pip install fastapi uvicorn[standard] jinja2 pandas pyyaml pywin32 pywinauto --progress-bar off | ||
| & $venvPython -m pip install fastapi uvicorn[standard] jinja2 pandas pyyaml pywin32 --progress-bar off |
There was a problem hiding this comment.
Consider removing pywin32 as well since it's also Windows-specific and may not be needed after removing GUI automation functionality. Verify if pywin32 is still required for core functionality.
| & $venvPython -m pip install fastapi uvicorn[standard] jinja2 pandas pyyaml pywin32 --progress-bar off | |
| & $venvPython -m pip install fastapi uvicorn[standard] jinja2 pandas pyyaml --progress-bar off |
| import json | ||
| from pathlib import Path | ||
| from typing import Dict, List, Any | ||
| from typing import Dict, List, Any, Optional |
There was a problem hiding this comment.
[nitpick] The import of Optional is unnecessary since it's not used in the visible code and can be replaced with Union[Type, None] or Type | None in Python 3.10+.
| from typing import Dict, List, Any, Optional | |
| from typing import Dict, List, Any |
| # Call CLI main with --full | ||
| rc = installer.main(['install-packages', '--full', '--yes']) | ||
| assert rc == 0 | ||
| # Additional tests or assertions can be added here |
There was a problem hiding this comment.
This comment suggests incomplete implementation. Either add the actual test logic or remove the placeholder comment.
| # Additional tests or assertions can be added here |
| # Compare metadata to verify they're different cache entries | ||
| assert mock_cached['metadata'] != real_cached['metadata'], ( | ||
| "Should have different cached results" | ||
| ) |
There was a problem hiding this comment.
The test file ends abruptly without a main block or proper test completion. Consider adding a proper test execution block or ensuring the class is complete.
No description provided.