Add matrix testing #13705
Add matrix testing #13705
Conversation
Changelog[uncommitted] (2023-09-13)Features
|
Currently, assertions are hand written as functions
that return `(ok bool, msg string)`.
Long-term, this will hurt readability
and has a higher risk of introducing bugs in testing
because conditions in assertions will be inverted
from what most people are used to
(checking the negative rather than the positive case).
This change introduces the ability to write these assertions
using a testing.T-compatible library like testify.
To do this, we introduce a new `*L` type analogous to `*T`
with an API similar to `testing.T`.
It implements more than sufficient methods to satisfy
Testify's TestingT interface, allowing its use with testify.
// The following is all that is necessary for Testify.
type L struct{ /* ... */ }
func (*L) Errorf(string, ...any)
func (*L) FailNow()
A function `WithL` is provided that constructs an `*L`,
runs the provided function, and returns the result.
func WithL(func(*L)) LResult
`L` should not be instantiated directly, without this function,
as this function handles the goroutine management necessary
to make `FailNow()` halt execution for `require`-based assertions.
Finally, this alters the contract for these tests, slightly.
Previously, the result of a test was represented roughly by the type:
type Result = enum {
Success,
Failure: { message: string }
}
That is, either a test is successful, or it failed and has a message.
With this change, the result is represented with the type,
type Result = {
messages: []string,
status: enum { Success, Failure }
}
That is, messages may be included regardless of success or failure.
This brings it more in line with how `go test` works:
`t.Log` and `t.Error` both log messages,
one just marks the test as failed.
As of right now, messages are sent back to the test runner
at the end of the language test.
In a future change, we should explore streaming these messages back
into the `testing.T`.
Adds an `L.Helper()` method that marks the caller as a helper function. Functions that call this will be skipped when gathering position information about where a log message comes from. This is especially desirable as all of testify's assertion functions call this method if it exists. Without this, all testify messages will start at assertions.go versus the actual test file.
|
Updates: |
|
Status for @Frassle when you're back:
|
| if !bytes.Equal(actualContents, expectedContents) { | ||
| // TODO: Find a way to show better diffs here | ||
| validations = append(validations, fmt.Sprintf("expected file %s does not match actual file", relativePath)) |
| for testName := range languageTests { | ||
| // Don't return internal tests | ||
| if strings.HasPrefix(testName, "internal-") { | ||
| continue | ||
| } | ||
| tests = append(tests, testName) | ||
| } |
There was a problem hiding this comment.
languageTests is a map, so the order of items in tests will be non-deterministic. Should this have a sort.Strings at the end?
There was a problem hiding this comment.
Order shouldn't really matter, but if it did matter we'd want declaration order. Could either change this to a list (but then we get slower lookups) or add a counter to each item in the array and sort by that here.
| @@ -0,0 +1,26 @@ | |||
| "use strict"; | |||
There was a problem hiding this comment.
Do we need to re-run this to delete this file?
There was a problem hiding this comment.
Yes, might need to fix the snapshot code to delete everything before writing new as well.
| func TestLanguage(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| os.Setenv("PULUMI_ACCEPT", "true") |
There was a problem hiding this comment.
No this was left over from working on this.
| @@ -242,7 +242,7 @@ func (host *nodeLanguageHost) GetRequiredPlugins(ctx context.Context, | |||
| // minor version, we will issue a warning to the user. | |||
| pulumiPackagePathToVersionMap := make(map[string]semver.Version) | |||
| plugins, err := getPluginsFromDir( | |||
| req.GetProgram(), | |||
| filepath.Join(req.Pwd, req.Program), | |||
There was a problem hiding this comment.
Could this change break anyone? I have a heightened sense for changes here now that this is the primary way plugins will be determined for Node.js projects.
There was a problem hiding this comment.
I don't think so.
Program is a relative path from the project root to the program, and we normally start language plugins with their working directory set to the project root.
This now just does that explicitly because when we're doing matrix testing the plugins working directory is the test directory.
We have quite a few tests for odd combinations of this in the integration tests and none of those flagged this as an issue.
Description
Adds the first pass of matrix testing.
Matrix testing allows us to define tests once in pulumi/pulumi via PCL and then run those tests against each language plugin to verify code generation and runtime correctness.
Rather than packing matrix tests and all the associated data and machinery into the CLI itself we define a new Go package at cmd/pulumi-test-lanaguage. This depends on pkg and runs the deployment engine in a unique way for matrix tests but it is running the proper deployment engine with a proper backend (always filestate, using $TEMP).
Currently only NodeJS is hooked up to run these tests, and all the code for that currently lives in sdk/nodejs/cmd/pulumi-language-nodejs/language_test.go. I expect we'll move that helper code to sdk/go/common and use it in each language plugin to run the tests in the same way.
This first pass includes 3 simple tests:
trueand `falseThese tests are themselves tested with a mock language runtime. This verifies the behavior of the matrix test framework for both correct and incorrect language hosts (that is some the mock language runtimes purposefully cause errors or compute the wrong result).
There are a number of things missing from from the core framework still, but I feel don't block getting this first pass merged and starting to be used.
stack.Previewand not pass a snapshot toassert.os.Stdout/Stderr. stdout/stderr streaming shows up in a load of other places as well so I'm thinking of a clean way to handle all of them together. Log message streaming we can probably do by just turning RunLanguageTest to a streaming grpc call.Checklist
make tidyto update any new dependenciesmake lintto verify my code passes the lint checkgofumptmake changelogand committed thechangelog/pending/<file>documenting my change