-
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 10 commits
bb283e6
8d87681
4a41449
15ea8ce
53629ca
d60973d
c7bf712
a164a90
5038929
6c5ce6d
78a9ff0
2ac149c
439b721
5579a83
a375e24
10a2441
c90e3ba
91da154
d0cd6b7
41a345e
b66f632
0f8622f
c7f86a4
9bfc747
8f83272
160e332
1ac7ff4
1579e2c
8535c08
3809a71
3e3b738
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,6 +1,186 @@ | ||||||
| subinclude("//test/build_defs") | ||||||
|
|
||||||
| sh_binary( | ||||||
| name = "build_statement_checker", | ||||||
| main = "build_statement_checker.sh", | ||||||
| ) | ||||||
|
|
||||||
| # Runs an export e2e test. It purposely tests for different aspect (e.g. BUILD files, sources) | ||||||
| # within the same test to avoid the overhead of exporting multiple times. | ||||||
DuBento marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| def please_export_e2e_test( | ||||||
DuBento marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| name:str, | ||||||
| export_targets:list, | ||||||
| repo:str, | ||||||
| cmd_on_export:list=[], | ||||||
| expect_output_contains:dict={}, | ||||||
| include_go:bool=False, | ||||||
| strict_check_build_stmt:bool=False, # requires match for all BUILD statements in exported file | ||||||
| EXPORT_DIR="plz-out/plzexport", | ||||||
| BUILD_FILE="BUILD_FILE"): | ||||||
| please_targets = ["//" + t for t in export_targets] | ||||||
| plz_command_targets = " ".join(please_targets) | ||||||
| tools = {} | ||||||
| config_override = [] | ||||||
| strict_opt = "--strict" if strict_check_build_stmt else "" | ||||||
|
|
||||||
| if include_go: | ||||||
| config_override += ["-o plugin.go.gotool:$TOOLS_GO"] | ||||||
| tools["GO"] = [CONFIG.GO.GO_TOOL] | ||||||
| config_override = " ".join(config_override) | ||||||
DuBento marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| plz_command = [ | ||||||
| # Export command | ||||||
| f"plz {config_override} export --output {EXPORT_DIR} {plz_command_targets}", | ||||||
| # Tests building the exported target which in turn ensures the sources are included | ||||||
| f"plz {config_override} --repo_root=$(plz query reporoot)/{EXPORT_DIR} build {plz_command_targets}", | ||||||
| ] + [f"plz {config_override} --repo_root=$(plz query reporoot)/{EXPORT_DIR} {cmd}" for cmd in cmd_on_export] | ||||||
|
|
||||||
| defer_cmd = [] | ||||||
| for export_target in export_targets: | ||||||
| file_path, target_name = export_target.split(":") | ||||||
| build_file = f"{file_path}/{BUILD_FILE}" | ||||||
|
|
||||||
| # Tests the contents of the exported BUILD file | ||||||
| defer_cmd += [f'$TOOLS_BUILD_STATEMENT_CHECKER {strict_opt} --exported "{EXPORT_DIR}/{build_file}" --original "{build_file}" --target "{target_name}"'] | ||||||
| tools["BUILD_STATEMENT_CHECKER"] = "//test/export:build_statement_checker" | ||||||
|
|
||||||
| if expect_output_contains: | ||||||
| updated_expect_output = {} | ||||||
| for key, val in expect_output_contains.items(): | ||||||
| updated_expect_output[f"{EXPORT_DIR}/{key}"] = val | ||||||
| expect_output_contains = updated_expect_output | ||||||
|
|
||||||
| return please_repo_e2e_test( | ||||||
| name = name, | ||||||
| defer_cmd = defer_cmd, | ||||||
| expect_output_contains = expect_output_contains, | ||||||
| plz_command = plz_command, | ||||||
| repo = repo, | ||||||
| tools = tools, | ||||||
| ) | ||||||
|
|
||||||
| # Export a target generated by a native genrule target and trim unused build statements. | ||||||
| please_export_e2e_test( | ||||||
DuBento marked this conversation as resolved.
Show resolved
Hide resolved
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", | ||||||
| export_targets = ["test_builtins:native_genrule"], | ||||||
| repo = "repo", | ||||||
| strict_check_build_stmt = True, | ||||||
| ) | ||||||
|
|
||||||
| # Export a target generated by a custom build def. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_simple_custom_target", | ||||||
| export_targets = ["test_custom_defs:simple_custom_target"], | ||||||
| repo = "repo", | ||||||
| # TODO Enable strict check after after support for trimming subincludes #3496 | ||||||
| # strict_check_build_stmt = True, | ||||||
| ) | ||||||
|
|
||||||
| # 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 //test_custom_defs:standard_children_custom_target#child", | ||||||
| ], | ||||||
| export_targets = ["test_custom_defs:standard_children_custom_target"], | ||||||
| repo = "repo", | ||||||
| # TODO Enable strict check after after support for trimming subincludes #3496 | ||||||
| # strict_check_build_stmt = True, | ||||||
| ) | ||||||
|
|
||||||
| # Export a target that depends on another target. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_deps", | ||||||
| export_targets = ["test_deps:dep2"], | ||||||
| repo = "repo", | ||||||
| ) | ||||||
|
|
||||||
| # Test multiple targets. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_multiple", | ||||||
| export_targets = [ | ||||||
| "test_multiple:target1", | ||||||
| "test_multiple:target2", | ||||||
| ], | ||||||
| repo = "repo", | ||||||
| ) | ||||||
|
|
||||||
| # 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", | ||||||
| expect_output_contains = { | ||||||
| # validating that export includes preloaded build def files | ||||||
| "build_defs/preloaded.build_defs": "preloaded_target", | ||||||
| "build_defs/BUILD_FILE": "preloaded_build_def", | ||||||
| }, | ||||||
| export_targets = ["test:native_not_including_preloaded"], | ||||||
| repo = "repo_preload", | ||||||
| strict_check_build_stmt = True, | ||||||
| ) | ||||||
|
|
||||||
| # Export a custom target from a repo that preloads the build def used. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_preload_use_preloaded_def", | ||||||
| export_targets = ["test:use_preloaded_def"], | ||||||
| repo = "repo_preload", | ||||||
| strict_check_build_stmt = True, | ||||||
| ) | ||||||
|
|
||||||
| # 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 |
||||||
| export_targets = ["test_go_bin:dummy"], | ||||||
| include_go = True, | ||||||
| repo = "repo_go", | ||||||
| strict_check_build_stmt = True, | ||||||
| ) | ||||||
|
|
||||||
| # Test native target with go third_party dependency. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_native_target_with_go_dep", | ||||||
| export_targets = ["test_go_dep:genrule_go_dep"], | ||||||
| include_go = True, | ||||||
| repo = "repo_go", | ||||||
| # TODO Enable strict check after after support for trimming subincludes #3496 | ||||||
| # strict_check_build_stmt = True, | ||||||
| ) | ||||||
|
|
||||||
| # Test go target with go third_party dependency. | ||||||
| please_export_e2e_test( | ||||||
| name = "export_go_target_with_go_dep", | ||||||
| export_targets = ["test_go_dep:bin_go_dep"], | ||||||
| include_go = True, | ||||||
| repo = "repo_go", | ||||||
| strict_check_build_stmt = True, | ||||||
| ) | ||||||
|
|
||||||
| # 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", | ||||||
| ) | ||||||
|
|
||||||
| # 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 //test_outputs:out_target", | ||||||
| repo = "repo", | ||||||
| ) | ||||||
|
|
||||||
| # Test --notrim flag. | ||||||
| please_repo_e2e_test( | ||||||
| name = "export_notrim", | ||||||
| defer_cmd = [ | ||||||
| 'test -f "plz-out/plzexport/test_notrim/file2.txt"', | ||||||
| 'test -f "plz-out/plzexport/test_notrim/BUILD_FILE"', | ||||||
| 'grep -q \'name = "unrelated"\' "plz-out/plzexport/test_notrim/BUILD_FILE"', | ||||||
| ], | ||||||
| plz_command = "plz export --notrim --output plz-out/plzexport //test_notrim:target1", | ||||||
| repo = "repo", | ||||||
| ) | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| strict_match=false | ||
| exported_file="" | ||
| original_file="" | ||
| target_name="" | ||
|
|
||
| while [[ "$#" -gt 0 ]]; do | ||
| case $1 in | ||
| --strict) | ||
| strict_match=true | ||
| shift | ||
| ;; | ||
| --exported) | ||
| exported_file=$2 | ||
| shift 2 | ||
| ;; | ||
| --original) | ||
| original_file=$2 | ||
| shift 2 | ||
| ;; | ||
| --target) | ||
| target_name=$2 | ||
| shift 2 | ||
| ;; | ||
| -*) | ||
| echo "Unknown option: $1" >&2 | ||
| exit 1 | ||
| ;; | ||
| *) | ||
| echo "Unknown argument: $1" >&2 | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| done | ||
|
|
||
| if [[ -z "$exported_file" ]] || [[ -z "$original_file" ]] || [[ -z "$target_name" ]]; then | ||
| echo "Usage: $0 [--strict] --exported <file> --original <file> --target <name>" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [[ ! -f "$exported_file" ]]; then | ||
| echo "$exported_file doesnt exist" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [[ ! -f "$original_file" ]]; then | ||
| echo "$original_file doesnt exist" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| readonly START_DELIM="# BUILD_STMT_START" | ||
| readonly END_DELIM="# BUILD_STMT_END" | ||
|
|
||
| # Using awk to extract statement blocks directly into an array. | ||
| blocks=() | ||
| while IFS= read -r -d '' block; do | ||
| blocks+=("$block") | ||
| done < <(awk -v id="$target_name" -v start_delim="$START_DELIM" -v end_delim="$END_DELIM" ' | ||
| BEGIN { in_block = 0; block = "" } | ||
| $0 == start_delim " " id { | ||
| in_block = 1; | ||
| block = ""; | ||
| next; | ||
| } | ||
| $0 == end_delim { | ||
| if (in_block) { | ||
| in_block = 0; | ||
| printf "%s\0", block; | ||
| } | ||
| next; | ||
| } | ||
| in_block { | ||
| block = block ? block "\n" $0 : $0 | ||
| } | ||
| ' "$original_file") | ||
|
|
||
| if [[ ${#blocks[@]} -eq 0 ]]; then | ||
| echo "Failed to pull original content for $target_name" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Ensure that ALL required blocks are present in the generated file | ||
| for block_content in "${blocks[@]}"; do | ||
| if ! grep -Fq "$block_content" "$exported_file"; then | ||
| printf '%s\n%s\n%s\n%s\n%s\n%s\n' \ | ||
| "BUILD statements mismatch" \ | ||
| "${exported_file} doesnt contain" \ | ||
| "${block_content}" \ | ||
| "---- it contains ----" \ | ||
| "$(cat "$exported_file")" \ | ||
| "---- EOF ----" >&2 | ||
| exit 1 | ||
| fi | ||
| done | ||
|
|
||
| # If --strict is enabled, ensure ONLY these blocks are present | ||
| # (ignoring all whitespace, newlines and comments). | ||
| if [[ "$strict_match" == true ]]; then | ||
| concatenated_blocks="" | ||
| for block_content in "${blocks[@]}"; do | ||
| concatenated_blocks="${concatenated_blocks}${block_content}" | ||
| done | ||
|
|
||
| stripped_blocks=$(echo -n "$concatenated_blocks" | sed 's/#.*//' | tr -d ' \t\n\r') | ||
| stripped_exported_file=$(sed 's/#.*//' "$exported_file" | tr -d ' \t\n\r') | ||
|
|
||
| if [[ "$stripped_blocks" != "$stripped_exported_file" ]]; then | ||
| printf '%s\n' "Strict match failed: exported file contains extra or out-of-order content." >&2 | ||
| printf '%s\n' "---- Expected (stripped) ----" >&2 | ||
| printf '%s\n' "$stripped_blocks" >&2 | ||
| printf '%s\n' "---- Got (stripped) ----" >&2 | ||
| printf '%s\n' "$stripped_exported_file" >&2 | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
|
||
| exit 0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| # BUILD_STMT_START native_genrule | ||
| genrule( | ||
| name = "native_genrule", | ||
| srcs = ["file.txt"], | ||
| outs = ["file.wordcount"], | ||
| cmd = "wc $SRCS > $OUT", | ||
| ) | ||
| # BUILD_STMT_END | ||
|
|
||
| # Du mmy target to be trimmed | ||
DuBento marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| genrule( | ||
| name = "dummy", | ||
| srcs = ["file.txt"], | ||
| outs = ["dummy"], | ||
| cmd = "cat $SRCS > $OUT", | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Test source file |
Uh oh!
There was an error while loading. Please reload this page.