-
-
Notifications
You must be signed in to change notification settings - Fork 405
Sync testing infrastructure with "template" assets #1388
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
Conversation
Some notes for the reviewer: Re: workflow run failures
Re: workflow configurationThe configurations of the These can provide a huge improvement to the contributor experience by avoiding the previous situation where a monster test suite runs on PRs that don't make any relevant changes. However, these filters require careful consideration to ensure that they are both as optimized as possible but also that they are not excluding any relevant files. It is not necessarily obvious which files are relevant. |
We have assembled a collection of reusable project assets: https://github.com/arduino/tooling-project-assets These will be used in the repositories of all Arduino tooling projects. Some minor improvements and standardizations have been made in the upstream "template" assets, and those are hereby introduced to this repository. Notable: - Configure paths filters to avoid unnecessary workflow runs - Increased parallelism - Improved maintainability
The sync with the template testing assets has resulted in new standardized names for some of the test runner tasks: - `test-unit` -> `go:test` - `test-integration` -> `go:test-integration`
Workflows look great to me, I especially like that test artifacts are created in a separate workflow and we don't have to wait for the whole test suite to run to build and upload them. The failing workflows are working correctly so we must fix those issues. I'll wait for those before approving. |
As of Go 1.7 the `golang.org/x/net/context` package is available in the standard library under the name `context`.
Result of running `go mod tidy` with Go 1.16.6.
…ry/discovery_client` module's dependencies Result of running `go mod tidy` with Go 1.16.6.
…odule's dependencies Result of running `go mod tidy` with Go 1.16.6.
…'s dependencies Result of running `go mod tidy` with Go 1.16.6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great 👍
Please check if the PR fulfills these requirements
before creating one)
our contributing guidelines
UPGRADING.md
has been updated with a migration guide (for breaking changes)Infrastructure update
We have assembled a collection of reusable project assets:
https://github.com/arduino/tooling-project-assets
These will be used in the repositories of all Arduino tooling projects.
Some improvements and standardizations have been made in the upstream "template" assets, but those have not been pulled into this repository.
Workflow is synced with the state of the art from upstream.
Notable:
Configure paths filters to avoid unnecessary workflow runs
Increased parallelism
Improved maintainability
Remove pointless attempt to upload nonexistent integration test coverage data to Codecov
Does this PR introduce a breaking change, and is
titled accordingly?
The sync with the template testing assets has resulted in new standardized names for some of the test runner tasks:
test-unit
->go:test
test-integration
->go:test-integration
The old tasks have been removed. This will disrupt the workflow of any developer who is accustomed to having them available.
This could be avoided by deprecating the tasks rather than doing an immediate removal. However, this removal unlikely to have a significant impact.
There is no change to the application's API.
Yes
I did not tackle syncing the integration test Python code with the templates:
The
run_command
test fixture from the "template" pytest code uses a list input. In order to use this, most of the many integration tests will need to be adapted. The rest of the infrastructure is not dependent on that sync, and this PR is already far too big, so I don't think it would be appropriate to tack the integration test code in this PR.