This TODO has been there since this
6ef7ef64d6 was merged to the main branch,
but the flag was always true, and the comment also tells us to set it to
true. Setting it to true is still the right thing, so let's just get rid
of the confusing comment.
Just a minor cleanup I noticed while working on some CI stuff.
I was watching the merge queue quite closely today, and it's continually
stuck waiting to run jobs on MacOS runners. However when the jobs are
running on them they tend to be faster than e.g. Windows runners, often
running the tests assigned to them in 2 min, vs. 8 min on windows.
In addition we're spending quite a bit of time to setup the actual
running of tests, and by spreading the tests out over multiple runners
that time multiplies. Given the macos runners are the most expensive
ones that probably also adds up.
Given all that, we can reduce the number of MacOS runners we're using,
while not trading that off against longer overall CI times, as those are
limited by slower jobs.
Unfortunately this results in some duplication in the ci job, but I
think the tradeoff here might be worth it.
(Also happy to hear if we think this isn't worth the extra messiness in
the GitHub actions files, as usually the merge queue isn't as bad as it
is today, so this isn't that much of an issue)
---------
Co-authored-by: Justin Van Patten <jvp@justinvp.com>
# Description
This is quite an awkward test because it depends on a secret from the
environment. However I don't really know how to do this otherwise since
we do need this SAS token, and that's a secret we need, so we have to
inject it through the environment.
/xref #15138
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [x] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
Add a test showing the problems we ran into when upgrading from
gocloud.dev 0.27.0 to 0.28.0 and 0.29.0. It's a little bit awkward,
since the issue only happens when upgrading form an encrypted key
generated with the old version of gocloud.dev to a newer version of
gocloud.dev.
Fixes https://github.com/pulumi/pulumi/issues/11986
---------
Co-authored-by: Thomas Gummerer <t.gummerer@gmail.com>
Fixes#14817
As a follow-up to #14804, we need to re-enable code coverage without
releases being built with coverage. #14804 includes a test in the
release job to catch `pulumi` being released with coverage data.
Also fixes the test to check that the pulumi binary is not built with
coverage.
Background:
Coverage was disabled on merge jobs in order to fix
[pulumi/pulumi#14799].
[pulumi/pulumi#14716] removed nightly coverage workflows in favor of
collecting coverage on merges [pulumi/pulumi#14727].
The binaries built with coverage were used in the downstream
prepare-release job causing a noticable bug [pulumi/pulumi#14799].
<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->
Fixes#13835
Change CodeCov to only upload on completely if all jobs succeed.
Consolidates CodeCov upload task in `ci-run-test.yml` to `ci.yml`.
There are extraneous commits that will be squashed before merging to
retain commit `55e783c997021d9c5fae2252708ad35f62eb20c6` displaying a
successful run for the reviewer.
Fixes https://github.com/pulumi/pulumi/issues/13826.
This brings python inline with node and go as being it's own module and
running codegen via gRPC.
Also includes some improvements to the node and go codegen interfaces
from review.
The CI test matrix generator expects every module that we want to test
to be provided explicitly.
Since the Go and Node hosts became independent Go modules,
we haven't been running their unit tests in CI.
Fortunately, the tests still pass today,
but we need to include these in the build.
**Overview**
This re-enables tracking of code coverage.
For Go, there are two kinds of coverage at play:
unit test and integration test coverage.
Unit tests follow the usual pattern of running
`go test -cover -coverprofile=whatever.cov`.
For integration tests, we use the new integration test profiling support
[added in Go 1.20](https://go.dev/testing/coverage/).
In short, the way it works is:
# Build a coverage instrumented binary:
go build -cover
# Set GOCOVERDIR to a directory and run the integration tests
# that will invoke this coverage-instrumented binary.
GOCOVERDIR=$(pwd)/coverage
go test ./tests
# $GOCOVERDIR will now be filled with coverage data
# from every invocation of the coverage-instrumented binary.
# Combine it into a single coverage file:
go tool covdata textfmt -i=$(GOCOVERDIR) -o=out.cov
# The resulting file can be uploaded to codecov as-is.
The above replaces the prior, partially working hacks we had in place
to get coverage-instrumented binaries with `go test -c`
and hijacking the TestMain.
**Notable changes**
- TestMain hijacking is deleted from the Pulumi CLI.
We no longer need this to build coverage-instrumented binaries.
- ProgramTest no longer tracks or passes PULUMI_TEST_COVERAGE_PATH
because the Pulumi binary no longer accepts a test.coverprofile flag.
This information is now in the GOCOVERDIR environment variable.
- We add an `enable-coverage` parameter to the `ci-build-binaries`
workflow to mirror some of the other workflows.
It will produce coverage-instrumented binaries if this is true.
These binaries are then used by `ci-run-test` which will set
`GOCOVERDIR` and merge the coverage results from it.
- Coverage configuration no longer counts tests, testdata,
and Protobuf-generated code against coverage.
- go-wrapper.sh:
Because we're no longer relying on the `go test -c` hack,
this no longer excludes Windows and language providers
from coverage tracking.
- go-test.py and go-wrapper.sh will include pulumi-language-go and
pulumi-language-nodejs in covered packages.
*Other changes*
- go-test.py:
Fixed a bug where `args` parameters added for coverage were ignored.
Note that this change DOES NOT track coverage for calls made to Pulumi
packages by plugins downloaded from external sources,
e.g. provider plugins. Arguably, that's out of scope of coverage
trackcing for the Pulumi repository.
Resolves#8615, #11419
We currently run all codegen tests in pkg/codegen/$lang
for every PR.
These tests take quite a while to run and lock up many GitHub workers
for this entire duration.
This change attempts to address this issue
by running codegen tests only for those PRs
that touch the codegen directories.
The machinery to make this work is roughly as follows:
- In the on-pr workflow, when we're figuring out what we're doing,
we check if we've changed codegen files.
We use [paths-filter] to do this.
- We decide whether we want to run codegen tests based on those files,
and pass that onto the test matrix generator.
- The test matrix generator filters out these packages
and their subpackages from the list of tests under consideration.
- Everything else proceeds as normal.
[paths-filter]: https://github.com/dorny/paths-filter
Things to note:
- The test-codegen input defaults to true.
All other invocations will run with codegen tests
so these will continue to run on merge.
Only PRs (from on-pr.yml) set it to false.
- Since the number of tests is remarkably smaller without these tests,
we can significantly reduce the number of partitions we use
for pkg/ unit tests.
This should alleviate pressure on GitHub workers further.
This is a pretty blunt approach to the problem.
If we wanted to be more targeted,
instead of filtering at the get-job-matrix.py level,
we could instead set an environment variable
and add t.Skips in {program,sdk,type}_driver
if that environment variable is set.
And we can still do that in the future
if we decide that maintaining this list is too much.
Resolves#12334
12028: Require linting before running unit, integ, and smoke tests. r=abhinav a=RobbieMcKinstry
<!---
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->
# Description
**Update:** With https://github.com/pulumi/pulumi/pull/12031 linting runs in about a minute.
Linting should be an extremely low-watermark requirement for evaluating build health. Blocking on it allows use to reduce the number of concurrent runners who are canceled early.
**Trade-offs:**
* This should delay CI time by the amount of time it takes to lint: _e.g._ CI will be ~5 minutes slower on the happy path.
* When a job is queued that fails a lint check, fewer runners will be soaked up just to fail lint checks. This will decrease the overall queue time across all builds.
* Ultimately, we're trading slower happy-path builds for smarter build scheduling.
* We can mitigate the linting bottleneck by speeding up the linting process (https://github.com/pulumi/pulumi/pull/12023).
<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->
This PR supports but isn't sufficient for https://github.com/pulumi/pulumi/issues/12019
## Checklist
**This PR is intended to impact CI only, and thus does not justify a CHANGELOG entry or a test.**
<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
<!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->
12043: sdk/go: Don't store DependsOn in a lossy form r=abhinav a=abhinav
The `DependsOn` and `DependsOnInputs` resource options
store their captured information on the `resourceOptions` struct
in a lossy format: they store function references.
This makes it impossible to go back to the original lists of resources
or resource array inputs for use cases like #11698.
As a step towards making this possible,
replace the stored closures with interfaces.
The implementations in the first commit
are a drop-in replacement for the prior behavior
with no logic changes whatsoever.
The second commit makes a minor optimization:
it adds URNs to the same set instead
of constantly allocating new sets and combining them afterwards.
Refs #11698
12048: ci/test-docs-generation: Attempt to use version-sets r=abhinav a=abhinav
Addresses the TODO and uses version-set generated by get-job-matrix
to decide which version of our various platforms should be used.
12069: test/integration: Capture component setup output r=abhinav a=abhinav
Issue #12050 would have been a lot easier to debug
if we had been capturing the output of the commands.
This changes runComponentSetup to execute bash with `-x`
so that every command in the shell script is logged before it's run.
This feeds that output into an `io.Writer` that logs to the testing.TB.
This way, if the test fails or times out,
we'll have a bunch of meaningful log output
associated with the tests that ran them.
Capturing the command output in a reliable way
required introduction of something slightly more complicated
than `strings.Split(\n)` because there's a good chance
that we'll have partial lines written to the writer.
The approach taken by the Writer here is:
it will log the input one line at a time usually,
but when it sees a partial line, it will buffer it in-memory
to be combined with the next write or flush.
Approach inspired by https://github.com/uber-go/zap/blob/v1.24.0/zapio/writer.go
Refs #12050
Co-authored-by: Robbie McKinstry <robbie@pulumi.com>
Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
In #11155, we pinned CI to macos-11
because we want to continue to support macOS 11.
This, however hsa created a new issue:
grpcio does not publish wheels for macos-11 anymore
and building it from source takes long enough to kill our CI
(see #12054#12050).
To fix the issue while continuing to support older macOS,
run CI on macos-latest, but leave build jobs and friends on macos-11.
Resolves#12054, #12050
Linting should be an extremely low-watermark requirement
for evaluating build health. Blocking on it allows use to reduce
the number of concurrent runners who are cancelled early.
12023: Speed Up Linting r=abhinav a=RobbieMcKinstry
<!---
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->
# Description
This PR unifies the Makerules for `lint_pkg`, `lint_sdk`, and `lint_tests` under a single `make lint` command. This improves lint performance by 30% because `golangci-lint` only has to read files into memory once instead of three times (even with a warm filesystem cache).
[This link shows the difference](https://app.warp.dev/block/AXj7EBwHqoxtzqfJ9ivzdL).
<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->
This PR is part of https://github.com/pulumi/pulumi/issues/12019 but is insufficient to close it.
**Note:** I had to add a `go.work` file to the top level directory to make this happen, which I expect isn't an issue since we have agreed we're not supporting Go 1.17 anymore.
## Checklist
**This PR affects build scripts only, and is not expected to impact the application.**
<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
<!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->
Co-authored-by: Robbie McKinstry <robbie@pulumi.com>
Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
After internal discussion, we determined "smoke" is a misleading
adjective for this category of tests. What we called "smoke tests"
are short integration tests for basic cross-platform functionality.
As a result, these are better named "acceptance" tests, since smoke
tests are intended to be a low water mark at the unit level to sniff
out bigger issues with the build as a whole.