Implement e2e tests for export functionality#3506
Implement e2e tests for export functionality#3506DuBento wants to merge 33 commits intothought-machine:masterfrom
Conversation
toastwaffle
left a comment
There was a problem hiding this comment.
Let's ensure that each test repo has at least one target that does not get exported (getting it as close as possible to the edge of being exported). You've got quite a few cases here where the expected repo is identical to the test repo, which doesn't prove anything for the "export as little as possible" goal of exporting. Bonus points for making the e2e enforce that the test repo and expected repo are different.
Also, what's the behaviour around comments in BUILD files - is it worth a test case which exposes that?
| config_override = "" | ||
| if include_go: | ||
| config_override += "-o plugin.go.gotool:$TOOLS_GO" | ||
| tools["GO"] = [CONFIG.GO.GO_TOOL] |
There was a problem hiding this comment.
is this necessary at all?
There was a problem hiding this comment.
If not included the tests fail with "go not found in the path" error.
My understanding with this is that we are injecting from the current go config (plz config of the caller) into the test repo, forwarding the go binary from //third_party/go:toolchain.
I based it on this test: https://sourcegraph.com/r/github.com/thought-machine/please/-/blob/test/proto_plugin/BUILD?L10-15
There was a problem hiding this comment.
Another possible alternative could be to specify a go_toolchain in the source_repo to make it self-contained and closer to a real plz repo. You were probably hinting at that from the beginning. I'll check if this is possible.
There was a problem hiding this comment.
Updated.
It works but let me know if this is what you were thinking, I'm slightly concern about tests starting to fail because of changes to go_toolchain (or versions) but I don't see that happening often.
build stmt checker now uses awk, supports more than one stmt associated with with a target and has a strict matching mode
Reverted changes to main test build def
This changes introduce small, targeted export test cases. The goal is to use these test for validating upcoming changes to the export logic.
please_export_e2e_testbuild def that is heavily inspired byplease_repo_e2e_testbut branches away to avoid injecting any base configuration files.test_repoand anexpected_repowith the entire files for testing and for validating the export.please_repo_e2e_testwere added to accommodate additional logic.