-
Notifications
You must be signed in to change notification settings - Fork 23
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
Test Azure CLI and templates #208
Conversation
Test that the `az` CLI is working and can login. Test that the `azure-${sdk}` templates work. This requires the following secrets in GHA: * ARM_CLIENT_ID * ARM_CLIENT_SECRET * ARM_TENANT_ID * ARM_SUBSCRIPTION_ID
for _, sdk := range sdksToTest { | ||
// python, typescript, ... | ||
testCases = append(testCases, testCase{sdk, map[string]string{}}) | ||
if sdk == "java" { |
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.
I don't want to change the containers in any way in this PR, so skipping some things that currently fail here, and below for azure-typescript.
t.Helper() | ||
v := os.Getenv(env) | ||
if v == "" { | ||
t.Fatalf("Required environment variable %q not set", env) |
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.
Should these be skipped, so tests can be run locally? Or I guess that maybe doesn't make a lot of sense for the docker containers? It's a pattern we used for other cloud tests.
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.
I was wondering about that, and I worried about a test not running because of a broken CI setup. I'd rather it fail loudly.
I was thinking that for running tests locally I could add something that runs esc run pulumi/providers.all -- test_command_here
. That gives you easy access to all the env vars.
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.
Yeah, this definitely makes sense during development.
The esc run
is also great, though I think it only works for pulumi employees (unless I'm misunderstanding esc).
Kindof an aside, but I'd love to get to the point where we skip no tests in CI, so we can assert that there are no skipped tests during the CI run, but sprinkle t.Skip()
more liberally through tests that don't necessarily work locally.
Either way, I'm happy to get what you have here now merged.
Pass through env variables for tests in the release workflow. This adds the changes from pulumi/pulumi-docker-containers#208 and pulumi/pulumi-docker-containers#213 to the release workflow.
Test that the
az
CLI is working and can login.Test that the
azure-${sdk}
templates work.This requires the following secrets in GHA:
Ref #209