Implement e2e tests for export functionality#3506
Implement e2e tests for export functionality#3506DuBento wants to merge 28 commits intothought-machine:masterfrom
Conversation
toastwaffle
left a comment
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?
test/export/BUILD
Outdated
|
|
||
| # # Test go binary target. | ||
| please_export_e2e_test( | ||
| name = "export_go_binary", |
There was a problem hiding this comment.
Other than the target being exported moving to a subdirectory, I don't see any behavioural difference between this and export_go_target_with_go_dep
| ) | ||
|
|
||
| # Test outputs export. | ||
| please_repo_e2e_test( |
There was a problem hiding this comment.
Why do you not use please_export_e2e_test here? I think both would be clearer as golden-master tests
test/export/BUILD
Outdated
| ) | ||
|
|
||
| # Export a target from a repo that preloads a build def. | ||
| # This test purposely doesn't use the custom def but checks that the source files are still included. |
There was a problem hiding this comment.
| # This test purposely doesn't use the custom def but checks that the source files are still included. | |
| # The exported target doesn't use the preloaded build def, but the preloaded build def is still present in the export. |
| 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?
|
|
||
| config_override = "" | ||
| if include_go: | ||
| config_override += "-o plugin.go.gotool:$TOOLS_GO" |
There was a problem hiding this comment.
can't we just put this in the .plzconfig in the source repo?
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
2 participants
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.