Sync test assets workflow with templates #227
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Due to the tests being located in a dedicated folder and written in Python, there are unique paths filter requirements for the GitHub Actions workflow that runs the project's integration tests. For this reason, and also to enable the unit test workflow to be applied to projects that use other integration testing approaches (i.e., none at all), it is better to use separate workflows to run unit tests vs. integration tests.
I have found that the approach of focusing each GitHub Actions workflows on a specific type of task can have some significant benefits - Improved efficiency due to being able to use a refined paths filter to prevent pointless runs of the workflow - Easier for contributors to interpret the results in the event of a failure - Easier to maintain - More likely to be applicable as a "template" to arbitrary projects This check really had no business being in the test runner workflow. It could be argued that it is suitable for inclusion in the "Check Go" workflow, but it does have unique paths filter requirements and is also not in that workflow's "template". So, at least for now, I am putting it in a dedicated workflow.
This process had no business being in the unit test runner workflow, not being related to the running of unit tests.
This is the template workflow filename, which is intended to serve as a unique identifier, and thus must be a bit more verbose.
The backticks command substitution syntax is discouraged in favor of the modern `$()` syntax for the reasons described here: http://mywiki.wooledge.org/BashFAQ/082 The modern syntax is already in use for other dynamic variables in the taskfile, so this change also provides consistency.
This is the naming convention established in the standardized template project assets.
A dedicated task has been added for installing the project's Python dependencies, and this should be used throughout the taskfile in place of redundant direct commands.
This allows the dependency tasks to run in parallel.
This is the naming convention established in the standardized template project assets.
These don't provide any value and make these assets more difficult to use as "templates" to be applied to any project.
This avoids the use of Windows "short paths" (e.g., C:\\Users\\RUNNER~1\\AppData\\Local) which may cause spurious test results.
Long commands, especially in this case where taskfile templating is involved, can be difficult to read and interpret. Adding strategic line breaks makes the structure of the command easy to follow.
Task has the capability for a default template value to be specified, which may be overridden by an environment variable set by the task user. The previous variable name chosen for this purpose was somewhat ambiguous.
Task's `default` template command uses the first value if the second is not defined. In this case, the second value was a variable defined by the workflow, so it was always defined! This made the default command and the first value useless. My interpretation is that the intent was, as is done successfully elsewhere, to allow the task user to make customizations by defining an environment variable from the command line which will override the default value. In order to accomplish that in a clean manner, I made the following changes: - Remove the frivolous taskfile variable, instead adding the flags directly to the template - Hard code the `-v` flag into the command - Add an override variable with a more relevant name
This will make it easier for the maintainers to sync fixes and improvements in either direction between the upstream "template" assets and their installation in this repository.
per1234
added
type: enhancement
Codecov Report
@@ Coverage Diff @@
## main #227 +/- ##
=======================================
Coverage 88.67% 88.67%
=======================================
Files 43 43
Lines 4143 4143
=======================================
Hits 3674 3674
Misses 358 358
Partials 111 111
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
silvanocerza
approved these changes
Aug 5, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have assembled a collection of reusable GitHub Actions workflows:
https://github.com/arduino/tooling-project-assets
These workflows will be used in the repositories of all Arduino tooling projects.
Some minor improvements and standardizations have been made in the upstream "template" workflow, and those are introduced to this repository via this pull request.