Skip to content
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.

Sign up for GitHub

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

Support execution constraints per exec group #13110

Closed
wants to merge 7 commits into from
Closed

Support execution constraints per exec group #13110

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
62ce436
Support setting properties under exec groups on platforms.
quval Dec 24, 2020
bb997b3
First apply the properties for the default exec group, then for others.
quval Feb 25, 2021
da8952d
Doc behaviour when applying properties for exec groups: properties fo…
quval Feb 25, 2021
1afeaf2
Merge branch 'master' of https://github.com/bazelbuild/bazel into tes…
quval Mar 2, 2021
903343e
Add test cases.
quval Mar 3, 2021
2ecd6e0
Test cases: execution properties for unknown execgroups
quval Mar 4, 2021
d6a4438
Align style with other tests
quval Mar 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions src/test/shell/bazel/platforms_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ EOF
bazel build --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Build failed"
grep "platform_key" out.txt || fail "Did not find the platform key"
grep "default_value" out.txt || fail "Did not find the default value"
grep "test_value" out.txt && fail "Used the test-action value when not testing"
(! grep "test_value" out.txt) || fail "Used the test-action value when not testing"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know this syntax existed. I see the benefits, but it's very different than all the other tests, so can you please change this back to the previous style, and use the same style in your new tests? I think consistency is better in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I couldn't figure out why, but one of the tests failed with that style (and I missed the test case with expected failure). I ended up removing it, as there is a positive test anyway.


bazel test --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Test failed"
grep "platform_key" out.txt || fail "Did not find the platform key"
Expand Down Expand Up @@ -353,7 +353,7 @@ EOF
bazel build --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Build failed"
grep "platform_key" out.txt || fail "Did not find the platform key"
grep "override_value" out.txt || fail "Did not find the overriding value"
grep "default_value" out.txt && fail "Used the default value"
(! grep "default_value" out.txt) || fail "Used the default value"

bazel test --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Test failed"
grep "platform_key" out.txt || fail "Did not find the platform key"
Expand Down Expand Up @@ -417,12 +417,55 @@ EOF
bazel build --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Build failed"
grep "platform_key" out.txt || fail "Did not find the platform key"
grep "override_value" out.txt || fail "Did not find the overriding value"
grep "default_value" out.txt && fail "Used the default value"
(! grep "default_value" out.txt) || fail "Used the default value"

bazel test --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Test failed"
grep "platform_key" out.txt || fail "Did not find the platform key"
grep "test_override" out.txt || fail "Did not find the overriding test-action value"
}

function test_platform_properties_only_applied_for_relevant_execgroups_cc_test() {
cat > a.cc <<'EOF'
int main() {}
EOF
cat > BUILD <<'EOF'
cc_test(
name = "a",
srcs = ["a.cc"],
)

platform(
name = "my_platform",
parents = ["@local_config_platform//:host"],
exec_properties = {
"platform_key": "default_value",
"unknown.platform_key": "unknown_value",
}
)
EOF
bazel test --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Build failed"
grep "platform_key" out.txt || fail "Did not find the platform key"
grep "default_value" out.txt || fail "Did not find the default value"
(! grep "unknown_value" out.txt) || fail "Used the value for an unknown execgroup"
}

function test_cannot_set_properties_for_irrelevant_execgroup_on_target_cc_test() {
cat > a.cc <<'EOF'
int main() {}
EOF
cat > BUILD <<'EOF'
cc_test(
name = "a",
srcs = ["a.cc"],
exec_properties = {
"platform_key": "default_value",
"unknown.platform_key": "unknown_value",
}
)
EOF
(! bazel test :a >& $TEST_log) || fail "Build passed when we expected an error"
grep "Tried to set properties for non-existent exec group" $TEST_log || fail "Did not complain about unknown exec group"
}

run_suite "platform mapping test"