automations:split tests into 2 workflows #558
Conversation
reduce waiting time for PR checks, codecov will merge both reports since we use flags
WalkthroughThis pull request introduces a new GitHub Actions workflow for end-to-end tests and modifies the existing unit tests workflow. The new Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
.github/workflows/unit_tests.yml (1)
85-85: Consider a more inclusive name for the codecov upload stepThe change from 'codecov-umbrella' to 'codecov-unittests' aligns with the focus on unit tests in this workflow. However, this workflow now includes both unit tests and integration tests.
Given that this workflow covers more than just unit tests, and considering that Codecov will merge reports from different workflows (as mentioned in the PR objectives), a more inclusive name might be appropriate. Consider changing it to:
- name: codecov-unittests + name: codecov-core-testsThis name would better reflect the comprehensive nature of the tests included in this workflow while distinguishing it from other potential test workflows.
.github/workflows/end2end_tests.yml (1)
69-84: LGTM: Well-configured test execution and coverage reporting with a minor suggestion.The test execution and coverage reporting setup is well-configured:
- End-to-end tests are run using pytest with appropriate coverage options.
- Coverage data is appended, allowing for combining results from different test runs.
- Coverage is uploaded to Codecov only for Python 3.9, avoiding duplicate reports.
- The Codecov configuration is comprehensive, including proper flagging and verbose output.
Minor suggestion: Consider adding a step to print the test results summary after the test execution. This can provide quick feedback in the GitHub Actions log. You can add this step after the "Run end2end tests" step:
- name: Print test summary if: always() run: | echo "Test Summary:" pytest --cache-clear --tb=native -v test/end2end --durations=0 -p no:sugar | tail -n 10This will always run (even if tests fail) and provide a quick summary of the test results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- .github/workflows/end2end_tests.yml (1 hunks)
- .github/workflows/unit_tests.yml (2 hunks)
- requirements/requirements.txt (1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/end2end_tests.yml
41-41: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
43-43: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
67-67: shellcheck reported issue in this script: SC2102:info:1:17: Ranges can only match single chars (mentioned due to duplicates)
(shellcheck)
.github/workflows/unit_tests.yml
68-68: shellcheck reported issue in this script: SC2102:info:2:14: Ranges can only match single chars (mentioned due to duplicates)
(shellcheck)
🔇 Additional comments (6)
requirements/requirements.txt (1)
9-9: LGTM. Verify compatibility with the updatedovos-utilsversion.The update to
ovos-utils>=0.1.0,<1.0.0is a minor version bump, which typically indicates new features or non-breaking changes. This change maintains compatibility with versions below 1.0.0, which is good for avoiding potential breaking changes.To ensure this change doesn't introduce any unexpected issues, please verify:
- The compatibility of this new version with the rest of the project.
- That the new version includes any necessary features or improvements for the updated testing workflows.
You can run the following script to check for any potential conflicts or issues:
.github/workflows/unit_tests.yml (3)
60-63: Excellent update to the unit tests execution!The addition of
--cov-flag=unittestsaligns well with the PR objective of splitting tests into two workflows. This flag will help in distinguishing coverage data from different test suites.The added comment about using
--cov-appendfor additional pytest invocations is very helpful. It provides clear guidance for maintaining consistent coverage reporting across multiple test runs, which is crucial for accurate overall coverage assessment.These changes contribute to better organization and clarity in the testing process, making it easier to manage and interpret test results.
73-73: Good update to the Padatious unit tests executionThe changes in this line are consistent with the overall strategy for coverage reporting:
- The addition of
--cov-appendensures that the coverage data from these tests is added to the existing report rather than overwriting it.- Using
--cov-flag=unittestsis appropriate here as these are indeed unit tests, albeit for a specific component (Padatious).These modifications contribute to a more comprehensive and accurate coverage report.
Line range hint
1-85: Overall, these changes significantly improve the testing workflowThe modifications to this workflow file align well with the PR objectives of splitting tests into two workflows. The changes contribute to a more organized and efficient testing process by:
- Adding appropriate coverage flags to distinguish between different test types.
- Ensuring consistent coverage reporting across multiple test invocations.
- Streamlining the workflow to focus on core and Padatious unit tests, along with integration tests.
These improvements will likely lead to faster PR checks and more accurate coverage reporting. Great job on enhancing the testing infrastructure!
🧰 Tools
🪛 actionlint
56-56: shellcheck reported issue in this script: SC2102:info:1:17: Ranges can only match single chars (mentioned due to duplicates)
(shellcheck)
68-68: shellcheck reported issue in this script: SC2102:info:2:14: Ranges can only match single chars (mentioned due to duplicates)
(shellcheck)
.github/workflows/end2end_tests.yml (2)
1-30: LGTM: Workflow name and triggers are well-defined.The workflow name is clear, and the triggers are appropriate for running end-to-end tests. The paths-ignore sections help avoid unnecessary test runs, and the manual dispatch option provides flexibility.
51-68: LGTM: Comprehensive test dependency and core repo installation.The installation process for test dependencies and the core repository is well-structured:
- Test dependencies are installed from
requirements/tests.txt.- Specific test skills are installed from local directories, ensuring consistency across different environments.
- The core repository is installed with the appropriate extras ('mycroft' and 'deprecated').
This setup should provide a reliable environment for running the end-to-end tests.
🧰 Tools
🪛 actionlint
67-67: shellcheck reported issue in this script: SC2102:info:1:17: Ranges can only match single chars (mentioned due to duplicates)
(shellcheck)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #558 +/- ##
==========================================
- Coverage 75.33% 74.81% -0.53%
==========================================
Files 15 15
Lines 3094 3049 -45
==========================================
- Hits 2331 2281 -50
- Misses 763 768 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
reduce waiting time for PR checks, codecov will merge both reports since we use flags
Summary by CodeRabbit
New Features
Bug Fixes
Chores
ovos-utilsto ensure compatibility with the latest features.