Makefile migration: make test
#636
Conversation
i-am-tom
added
the
impact/no-changelog-required
i-am-tom
force-pushed
the
i-am-tom/make-test-sdk
branch
from
89b2431 to
467df03
Compare
i-am-tom
changed the title
Makefile migration: make test-sdkMakefile migration: make test
| test: test_conformance test_integration test_sdk | ||
|
|
||
| .PHONY: test_conformance | ||
| test_conformance: build clean |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
|
||
| .PHONY: test_integration | ||
| test_integration: build clean | ||
| cd integration_tests && gotestsum -- $(GO_TEST_FILTER_FLAG) --parallel 1 --timeout 60m ./... |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This and the conformance test target are both unchanged from the previous behaviour, I think? Or is this a TODO for a future PR?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I'm on ops this week, so I'm happy to fix this in a follow-up
tgummerer
left a comment
tgummerer
left a comment
There was a problem hiding this comment.
Personally I'd have cleaned the targets up, but if we just want a bug for bug translation from the F# bits, I think this is fine.
2 participants
This moves
test_sdkout of the F#, and puts the pre-existing test commands under an umbrellamake test.