Skip to content

Makefile migration: make test #636

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.

Sign up for GitHub

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

Merged
i-am-tom merged 4 commits into from
Jun 25, 2025
Merged

Makefile migration: make test #636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
467df03
Make it so
i-am-tom Jun 11, 2025
01fbad1
Docs
i-am-tom Jun 25, 2025
c1105d6
Merge branch 'main' into i-am-tom/make-test-sdk
i-am-tom Jun 25, 2025
547deba
Well done Tom
i-am-tom Jun 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ jobs:
with:
allowed-changes: ./global.json
- name: Test Pulumi SDK
run: dotnet run test-sdk coverage
run: dotnet run test_sdk_coverage
- name: Test Pulumi Automation SDK
run: dotnet run test-automation-sdk coverage
- name: Upload coverage data
Expand Down
30 changes: 25 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
GO := go

# Try to get the dev version using changie, otherwise fall back
FALLBACK_DEV_VERSION := 3.0.0-dev.0
DEV_VERSION := $(shell if command -v changie > /dev/null; then changie next patch -p dev.0; else echo "$(FALLBACK_DEV_VERSION)"; fi)
SDK_VERSION := $(shell cd pulumi-language-dotnet && sed -n 's/^.*github\.com\/pulumi\/pulumi\/sdk\/v3 v//p' go.mod)

GO_TEST_FILTER_FLAG := $(if $(TEST_FILTER),-run $(TEST_FILTER))
DOTNET_TEST_FILTER_FLAG := $(if $(TEST_FILTER),--filter $(TEST_FILTER))

.PHONY: install
install:
cd pulumi-language-dotnet && ${GO} install \
Expand Down Expand Up @@ -96,8 +98,26 @@ lint_integration_tests: format_integration_tests_check
lint_integration_tests_fix: format_integration_tests
cd integration_tests && golangci-lint run $(GOLANGCI_LINT_ARGS) --fix --config ../.golangci.yml --timeout 5m --path-prefix integration_tests

test_integration:: build
cd integration_tests && gotestsum -- --parallel 1 --timeout 60m ./...
.PHONY: test
test: test_conformance test_integration test_sdk

.PHONY: test_conformance
test_conformance: build clean
Copy link
Contributor

@tgummerer tgummerer Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this depend on the clean target? We shouldn't need a clean build for this. Also I think that means it runs clean before it runs the tests with I think would be wrong.

I see that the F# makefile does that in some cases, but I think it shouldn't be necessary, and ideally we make things better here, so we don't need to whole clean -> restore dance.

Copy link
Contributor Author

@i-am-tom i-am-tom Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a Chesterton's fence - like I said in the format/lint PR, I think the best approach is to migrate F# to Make as directly as possible, and then worry about this in a follow-up

Copy link
Contributor

@tgummerer tgummerer Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I guess clean only cleans the SDK bits, which is confusing, but ok. At least we're not cleaning everything we just build here I guess. It's still weird that we clean this. Personally I'd prefer to make things better while we are here, since later cleanups are often forgotten, but I guess it is what we were running before.

Copy link
Contributor Author

@i-am-tom i-am-tom Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that cleanups get forgotten, but also I'm very keen to keep the scope of these changes very strict as there are plenty of other "quick fixes" I'd love to be adding as I go - I've no doubt this will get revisited when I get back to my CI endeavours, but it seemed like the problem with my original approach in pulumi/pulumi was trying to do too many things at once, so I'm still inclined to say this is out of scope for this PR

cd pulumi-language-dotnet && gotestsum -- $(GO_TEST_FILTER_FLAG) --timeout 60m ./...

.PHONY: test_integration
test_integration: build clean
cd integration_tests && gotestsum -- $(GO_TEST_FILTER_FLAG) --parallel 1 --timeout 60m ./...
Copy link
Contributor

@tgummerer tgummerer Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for this and the conformance tests above, we need to set up $PATH, so it uses the binary we just built. Right now it still uses the binary from the regular $PATH, so depending on build will make it build the language host, but then not actually use it.

Copy link
Contributor Author

@i-am-tom i-am-tom Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the conformance test target are both unchanged from the previous behaviour, I think? Or is this a TODO for a future PR?

Copy link
Contributor

@tgummerer tgummerer Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's equally broken before as it is now. So if you want just a bug for bug translation, I guess this works.

Copy link
Contributor Author

@i-am-tom i-am-tom Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on ops this week, so I'm happy to fix this in a follow-up


.PHONY: test_sdk
test_sdk: build clean
cd sdk && dotnet restore --no-cache
cd sdk/Pulumi.Tests && dotnet test --configuration Release $(DOTNET_TEST_FILTER_FLAG)

.PHONY: test_coverage
test_coverage: test_sdk_coverage

test_conformance:: build
cd pulumi-language-dotnet && gotestsum -- --timeout 60m ./...
.PHONY: test_sdk_coverage
test_sdk_coverage: clean
cd sdk && dotnet restore --no-cache
cd sdk/Pulumi.Tests && dotnet test --configuration Release $(DOTNET_TEST_FILTER_FLAG) -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput=../../coverage/coverage.pulumi.xml
16 changes: 8 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,14 @@ And finally, `pulumi preview` and `pulumi up` as you would any other Pulumi proj
Then you can run one of the following commands:

```bash
# Run all tests
make test [TEST_FILTER=testName]

# Build the Pulumi SDK
make build_sdk

# Running tests for the Pulumi SDK
dotnet run test-sdk
# Running tests for the Pulumi SDK [or a specific test]
make test_sdk [TEST_FILTER=testName]

# Running tests for the Pulumi Automation SDK
dotnet run test-automation-sdk
Expand All @@ -97,11 +100,8 @@ dotnet run test-language-plugin
# List all integration tests
go test -C integration_tests -list=. | grep ^Test

# Run a specific integration test
dotnet run integration test <testName>

# Run all integration tests
dotnet run all-integration-tests
# Run all integration tests [or a specific test]
make test_integration [TEST_FILTER=testName]

# Format the code
make format_fix
Expand All @@ -113,7 +113,7 @@ make format

When running integration tests via an IDE like Goland or VSCode, you want the Pulumi CLI to use the `pulumi-language-dotnet` plugin from this repository, not the one that comes bundled with your Pulumi CLI. To do this, in your terminal `dotnet run build-language-plugin` or simply `cd pulumi-language-dotnet && go build`.

Alternatively, you can run `dotnet run integration test <testName>` or `dotnet run all-integration-tests` which will build the language plugin for you just before running the tests.
Alternatively, you can run `make test_integration [TEST_FILTER=testName]` which will build the language plugin for you just before running the tests.

## Public API Changes

Expand Down
12 changes: 6 additions & 6 deletions build/Program.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,12 @@ let testLanguagePlugin() =
then failwith "Testing pulumi-language-dotnet failed"

let testPulumiSdk coverage =
cleanSdk()
restoreSdk()
printfn "Testing Pulumi SDK"
let coverageArgs = if coverage then $" -p:CollectCoverage=true -p:CoverletOutputFormat=cobertura -p:CoverletOutput={coverageDir}/coverage.pulumi.xml" else ""
if Shell.Exec("dotnet", "test --configuration Release" + coverageArgs, pulumiSdkTests) <> 0
then failwith "tests failed"
printfn "Deprecated: calling `make test_sdk%s` instead" (if coverage then "_coverage" else "")
let target = if coverage then "test_sdk_coverage" else "test_sdk"
let cmd = Cli.Wrap("make").WithArguments(target).WithWorkingDirectory(repositoryRoot)
let output = cmd.ExecuteAsync().GetAwaiter().GetResult()
if output.ExitCode <> 0 then
failwith "tests failed"

let testPulumiAutomationSdk coverage =
cleanSdk()
Expand Down
Loading