-
Notifications
You must be signed in to change notification settings - Fork 212
Implement e2e tests for export functionality #3506
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
base: master
Are you sure you want to change the base?
Changes from all commits
bb283e6
8d87681
4a41449
15ea8ce
53629ca
d60973d
c7bf712
a164a90
5038929
6c5ce6d
78a9ff0
2ac149c
439b721
5579a83
a375e24
10a2441
c90e3ba
91da154
d0cd6b7
41a345e
b66f632
0f8622f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,124 @@ | ||||||
| subinclude("//test/build_defs") | ||||||
| filegroup( | ||||||
| name = "export_e2e_test_build_def", | ||||||
| srcs = ["please_export_e2e_test.build_defs"], | ||||||
| ) | ||||||
|
|
||||||
| subinclude(":export_e2e_test_build_def") | ||||||
|
|
||||||
| # Export a target generated by a native genrule target and trim unused | ||||||
| # build statements. | ||||||
| please_export_e2e_test( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please could you move each of these |
||||||
| name = "export_native_genrule", | ||||||
| dir = "test_builtins", | ||||||
| export_targets = ["//:native_genrule"], | ||||||
| ) | ||||||
|
|
||||||
| # Export a target generated by a custom build def. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_simple_custom_target", | ||||||
| dir = "test_custom_def", | ||||||
| export_targets = ["//:simple_custom_target"], | ||||||
| ) | ||||||
|
|
||||||
| # Export a target generated by a custom build def defined in the same | ||||||
| # BUILD file. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_custom_target_in_file", | ||||||
| dir = "test_custom_in_file_def", | ||||||
| export_targets = ["//:simple_custom_target"], | ||||||
| ) | ||||||
|
|
||||||
| # Export a target generated by a custom build def. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_standard_children_custom_target", | ||||||
| cmd_on_export = [ | ||||||
| # Child of build def | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's avoid referring to children, as that's not well-defined terminology. What you're essentially capturing here is that a target A created in a build def alongside the target B that we're trying to export will result in both A and B being buildable |
||||||
| "build //:standard_children_custom_target#child", | ||||||
| ], | ||||||
| dir = "test_custom_def_children", | ||||||
| export_targets = ["//:standard_children_custom_target"], | ||||||
| ) | ||||||
|
|
||||||
| # Export a target that depends on another target. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_deps", | ||||||
| dir = "test_deps", | ||||||
| export_targets = ["//:dep2"], | ||||||
| ) | ||||||
|
|
||||||
| # Test multiple targets. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_multiple_targets", | ||||||
| dir = "test_multiple_targets", | ||||||
| export_targets = [ | ||||||
| "//:target1", | ||||||
| "//:target2", | ||||||
| ], | ||||||
| ) | ||||||
|
|
||||||
| # 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. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| please_export_e2e_test( | ||||||
| name = "export_preload_genrule", | ||||||
| dir = "test_preload", | ||||||
| export_targets = ["//:native_target"], | ||||||
| ) | ||||||
|
|
||||||
| # Export a custom target from a repo that preloads the build def used. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_preload_use_preloaded_def", | ||||||
| dir = "test_use_preloaded", | ||||||
| export_targets = ["//:use_preloaded_def"], | ||||||
| ) | ||||||
|
|
||||||
| # # Test go binary target. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_go_binary", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other than the target being exported moving to a subdirectory, I don't see any behavioural difference between this and |
||||||
| dir = "test_go_binary", | ||||||
| export_targets = ["//pkg:go_bin"], | ||||||
| include_go = True, | ||||||
| ) | ||||||
|
|
||||||
| # Test native target with go third_party dependency. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_native_target_with_go_dep", | ||||||
| dir = "test_native_target_with_go_dep", | ||||||
| export_targets = ["//:genrule_go_dep"], | ||||||
| include_go = True, | ||||||
| ) | ||||||
|
|
||||||
| # Test go target with go third_party dependency. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_go_target_with_go_dep", | ||||||
| dir = "test_go_bin_with_go_dep", | ||||||
| export_targets = ["//:bin_go_dep"], | ||||||
| include_go = True, | ||||||
| ) | ||||||
|
|
||||||
| # Test outputs export. | ||||||
| please_repo_e2e_test( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you not use |
||||||
| name = "export_outputs", | ||||||
| defer_cmd = [ | ||||||
| 'test -f "plz-out/plzexport/some_output.txt"', | ||||||
| 'test ! -f "plz-out/plzexport/BUILD_FILE"', | ||||||
| ], | ||||||
| plz_command = "plz export outputs --output plz-out/plzexport //:out_target", | ||||||
| repo = "test_outputs", | ||||||
| ) | ||||||
|
|
||||||
| # Test --notrim flag. | ||||||
| please_repo_e2e_test( | ||||||
| name = "export_notrim", | ||||||
| defer_cmd = [ | ||||||
| 'test -f "plz-out/plzexport/file2.txt"', | ||||||
| 'test -f "plz-out/plzexport/BUILD_FILE"', | ||||||
| 'grep -q \'name = "unrelated"\' "plz-out/plzexport/BUILD_FILE"', | ||||||
| ], | ||||||
| plz_command = "plz export --notrim --output plz-out/plzexport //:target1", | ||||||
| repo = "test_notrim", | ||||||
| ) | ||||||
|
|
||||||
| # Generic catch-all test on internal repo. | ||||||
| plz_e2e_test( | ||||||
| name = "export_src_please_test", | ||||||
| cmd = "plz export --output plz-out/plzexport //src/core && plz --repo_root=$(plz query reporoot)/plz-out/plzexport build //src/core", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| subinclude("//test/build_defs:build_defs") | ||
|
|
||
| def please_export_e2e_test( | ||
| name:str, | ||
| dir:str, | ||
| export_targets:list, | ||
| cmd_on_export:list=[], | ||
| include_go:bool=False): | ||
| """ | ||
| Runs an export e2e test. To use it, create a directory with both a `test_repo` with the targets | ||
| to be exported and an `expected_repo` with the expected resulting files. | ||
| The validation steps for the e2e test are: | ||
| * It performs a repo-wide diff between the exported files and the `expected_repo` | ||
| * Builds the exported target to ensure the exported repo, and consequently the | ||
| `expected_repo`, include valid sources and dependencies to build the target. | ||
| * Optionally you can specify additional commands to run on the exported directory by | ||
| passing them to `cmd_on_export`. | ||
|
|
||
| Args: | ||
| name (str): Name of the test. | ||
| dir (str): A relative path to the directory containing `test_repo` and `expected_repo`. | ||
| export_targets (list): Targets to export as part of the test. | ||
| cmd_on_export (list): Optional. Additional commands to run on the exported directory. | ||
| include_go (bool): Optional. A flag to include Go dependencies in the e2e test. Use when | ||
| testing repos that include Go targets. | ||
| """ | ||
| EXPORT_DIR = "plz-out/plzexport" | ||
| test_repo = f"{dir}/test_repo" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can we call this |
||
| expected_repo = f"{dir}/expected_repo" | ||
| exported_repo = f"$DATA_TEST_REPO/{EXPORT_DIR}" | ||
| data = {} | ||
| tools = {} | ||
|
|
||
| targets = " ".join(export_targets) | ||
|
|
||
| config_override = "" | ||
| if include_go: | ||
| config_override += "-o plugin.go.gotool:$TOOLS_GO" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can't we just put this in the |
||
| tools["GO"] = [CONFIG.GO.GO_TOOL] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this necessary at all? |
||
|
|
||
| test_cmd = [ | ||
| # Export | ||
| f'plz {config_override} --repo_root="$DATA_TEST_REPO" export --output "{EXPORT_DIR}" {targets}', | ||
| # Golden-Master validation with expected repo. Done before any building to avoid plz-out | ||
| f'diff -r "{exported_repo}" "$DATA_EXPECTED_REPO"', | ||
| # Tests building the exported target which in turn ensures the sources are included | ||
| f'plz {config_override} --repo_root="{exported_repo}" build {targets}', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it make sense for this to just be |
||
| ] + [f'plz {config_override} --repo_root="{exported_repo}" {cmd}' for cmd in cmd_on_export] | ||
|
|
||
| test_cmd = [cmd.replace("plz ", "$TOOLS_PLEASE ") for cmd in test_cmd] | ||
| test_cmd = " && ".join(test_cmd) | ||
|
|
||
| data["TEST_REPO"] = [test_repo] | ||
| data["EXPECTED_REPO"] = [expected_repo] | ||
|
|
||
| tools["PLEASE"] = ["//package:installed_files|please"] | ||
|
|
||
| return gentest( | ||
| name = name, | ||
| data = data, | ||
| labels = ["plz_e2e_test", "e2e"], | ||
| no_test_output = True, | ||
| sandbox = False, | ||
| test_cmd = test_cmd, | ||
| test_tools = tools, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| [Parse] | ||
|
|
||
| BuildFileName = BUILD_FILE |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| genrule( | ||
| name = "native_genrule", | ||
| srcs = ["file.txt"], | ||
| outs = ["file.wordcount"], | ||
| cmd = "wc $SRCS > $OUT", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Test source file |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| [Parse] | ||
|
|
||
| BuildFileName = BUILD_FILE |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| genrule( | ||
| name = "native_genrule", | ||
| srcs = ["file.txt"], | ||
| outs = ["file.wordcount"], | ||
| cmd = "wc $SRCS > $OUT", | ||
| ) | ||
|
|
||
| genrule( | ||
| name = "dummy_target_to_be_trimmed", | ||
| srcs = ["dummy.txt"], | ||
| outs = ["dummy"], | ||
| cmd = "cat $SRCS > $OUT", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Test source file |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| [Parse] | ||
|
|
||
| BuildFileName = BUILD_FILE |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| subinclude("//build_defs:simple_build_def") | ||
|
|
||
| simple_custom_target( | ||
| name = "simple_custom_target", | ||
| srcs = ["file.txt"], | ||
| outs = ["file_simple.out"], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| filegroup( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either this should contain an entry for dummy.build_defs, or dummy.build_defs shouldn't exist |
||
| name = "simple_build_def", | ||
| srcs = ["simple.build_defs"], | ||
| visibility = ["PUBLIC"], | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| def simple_custom_target( | ||
| name:str, | ||
| srcs:list=[], | ||
| outs:list=[]): | ||
| return genrule( | ||
| name = name, | ||
| srcs = srcs, | ||
| outs = outs, | ||
| cmd = "cat $SRCS > $OUT", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Test source file |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| [Parse] | ||
|
|
||
| BuildFileName = BUILD_FILE |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| subinclude("//build_defs:simple_build_def") | ||
|
|
||
| simple_custom_target( | ||
| name = "simple_custom_target", | ||
| srcs = ["file.txt"], | ||
| outs = ["file_simple.out"], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| filegroup( | ||
| name = "simple_build_def", | ||
| srcs = ["simple.build_defs"], | ||
| visibility = ["PUBLIC"], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| def dummy_target( | ||
| name:str, | ||
| srcs:list=[], | ||
| outs:list=[]): | ||
| return genrule( | ||
| name = name, | ||
| srcs = srcs, | ||
| outs = outs, | ||
| cmd = "echo dummy > $OUT", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| def simple_custom_target( | ||
| name:str, | ||
| srcs:list=[], | ||
| outs:list=[]): | ||
| return genrule( | ||
| name = name, | ||
| srcs = srcs, | ||
| outs = outs, | ||
| cmd = "cat $SRCS > $OUT", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Test source file |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| [Parse] | ||
|
|
||
| BuildFileName = BUILD_FILE |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| subinclude("//build_defs:standard_children_build_def") | ||
|
|
||
| standard_children_custom_target( | ||
| name = "standard_children_custom_target", | ||
| srcs = ["file.txt"], | ||
| outs = ["file_standard.out"], | ||
| outs_child = ["file_child.out"], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| filegroup( | ||
| name = "standard_children_build_def", | ||
| srcs = ["standard_children.build_defs"], | ||
| visibility = ["PUBLIC"], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| def standard_children_custom_target( | ||
| name:str, | ||
| srcs:list=[], | ||
| outs:list=[], | ||
| outs_child:list=[]): | ||
| genrule( | ||
| name = f"{name}#child", | ||
| srcs = srcs, | ||
| outs = outs_child, | ||
| cmd = "echo 'child' > $OUT && cat $SRCS >> $OUT ", | ||
| ) | ||
| return genrule( | ||
| name = name, | ||
| srcs = srcs, | ||
| outs = outs, | ||
| cmd = "echo 'main' > $OUT && cat $SRCS >> $OUT ", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Test source file |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| [Parse] | ||
|
|
||
| BuildFileName = BUILD_FILE |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| subinclude("//build_defs:standard_children_build_def") | ||
|
|
||
| standard_children_custom_target( | ||
| name = "standard_children_custom_target", | ||
| srcs = ["file.txt"], | ||
| outs = ["file_standard.out"], | ||
| outs_child = ["file_child.out"], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| filegroup( | ||
| name = "standard_children_build_def", | ||
| srcs = ["standard_children.build_defs"], | ||
| visibility = ["PUBLIC"], | ||
| ) |
Uh oh!
There was an error while loading. Please reload this page.