Skip to content

wasm: exclude C++ tests on all unsupported platforms. #19615

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

Merged
lizan merged 2 commits into from
Jan 24, 2022
Merged

wasm: exclude C++ tests on all unsupported platforms. #19615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e428f23
wasm: exclude C++ tests on all unsupported platforms.
PiotrSikora Jan 19, 2022
2233889
review: remove double-negative.
PiotrSikora Jan 21, 2022
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
26 changes: 25 additions & 1 deletion bazel/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ config_setting(
)

config_setting(
name = "wasm_none",
name = "wasm_disabled",
values = {"define": "wasm=disabled"},
)

Expand Down Expand Up @@ -591,6 +591,30 @@ selects.config_setting_group(
],
)

selects.config_setting_group(
name = "not_x86",
match_any = [
":darwin_arm64",
":ios_arm64",
":ios_arm64e",
":ios_armv7",
":ios_armv7s",
":ios_i386",
":linux_aarch64",
":linux_mips64",
":linux_ppc",
":linux_s390x",
],
)

selects.config_setting_group(
name = "not_x86_or_wasm_disabled",
match_any = [
":not_x86",
":wasm_disabled",
],
)

alias(
name = "remote_jdk11",
actual = "@bazel_tools//tools/jdk:remote_jdk11",
Expand Down
10 changes: 4 additions & 6 deletions bazel/envoy_select.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,15 @@ def envoy_select_hot_restart(xs, repository = ""):
# and the ability to build tests using Proxy-Wasm C++ SDK on the current platform.
def envoy_select_wasm_cpp_tests(xs):
return select({
"@envoy//bazel:darwin_arm64": [],
"@envoy//bazel:linux_aarch64": [],
"@envoy//bazel:wasm_none": [],
"@envoy//bazel:not_x86_or_wasm_disabled": [],
"//conditions:default": xs,
})

# Selects the given values depending on the Wasm runtimes enabled in the current build,
# and the ability to build tests using Proxy-Wasm Rust SDK on the current platform.
def envoy_select_wasm_rust_tests(xs):
return select({
"@envoy//bazel:wasm_none": [],
"@envoy//bazel:wasm_disabled": [],
"//conditions:default": xs,
})

Expand All @@ -64,7 +62,7 @@ def envoy_select_wasm_v8(xs):
"@envoy//bazel:wasm_wamr": [],
"@envoy//bazel:wasm_wasmtime": [],
"@envoy//bazel:wasm_wavm": [],
"@envoy//bazel:wasm_none": [],
"@envoy//bazel:wasm_disabled": [],
"//conditions:default": xs, # implicit default (v8)
})

Expand All @@ -75,7 +73,7 @@ def envoy_select_wasm_v8_bool():
"@envoy//bazel:wasm_wamr": False,
"@envoy//bazel:wasm_wasmtime": False,
"@envoy//bazel:wasm_wavm": False,
"@envoy//bazel:wasm_none": False,
"@envoy//bazel:wasm_disabled": False,
"//conditions:default": True, # implicit default (v8)
})

Expand Down
4 changes: 2 additions & 2 deletions test/extensions/access_loggers/wasm/config_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ TEST_P(WasmAccessLogConfigTest, CreateWasmFromEmpty) {
}

TEST_P(WasmAccessLogConfigTest, CreateWasmFromWASM) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
Copy link
Copy Markdown
Member

@lizan lizan Jan 20, 2022

Choose a reason for hiding this comment

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

Can we define our own macros in bazel rules (like ENVOY_WASM_ENABLE_CPP_TESTS) for these? So when we change how these are enabled we don't need to change all of the tests.

Copy link
Copy Markdown
Contributor Author

@PiotrSikora PiotrSikora Jan 21, 2022

Choose a reason for hiding this comment

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

I did something similar (#19641), but it's quite a big refactor of tests and the core change here is completely lost in the refactoring, so I'd prefer if we could merge this as-is for the fixed #define, and then get the refactored tests merged.

Copy link
Copy Markdown
Member

@lizan lizan Jan 24, 2022

Choose a reason for hiding this comment

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

ok that sounds good to me

// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand Down
32 changes: 16 additions & 16 deletions test/extensions/bootstrap/wasm/config_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ INSTANTIATE_TEST_SUITE_P(Runtimes, WasmFactoryTest,
Envoy::Extensions::Common::Wasm::runtime_values);

TEST_P(WasmFactoryTest, CreateWasmFromWasm) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -90,8 +90,8 @@ TEST_P(WasmFactoryTest, CreateWasmFromWasm) {
}

TEST_P(WasmFactoryTest, CreateWasmFromWasmPerThread) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -105,8 +105,8 @@ TEST_P(WasmFactoryTest, CreateWasmFromWasmPerThread) {
}

TEST_P(WasmFactoryTest, MissingImport) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -122,8 +122,8 @@ TEST_P(WasmFactoryTest, MissingImport) {
}

TEST_P(WasmFactoryTest, UnspecifiedRuntime) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -136,8 +136,8 @@ TEST_P(WasmFactoryTest, UnspecifiedRuntime) {
}

TEST_P(WasmFactoryTest, UnknownRuntime) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -149,8 +149,8 @@ TEST_P(WasmFactoryTest, UnknownRuntime) {
}

TEST_P(WasmFactoryTest, StartFailed) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -165,8 +165,8 @@ TEST_P(WasmFactoryTest, StartFailed) {
}

TEST_P(WasmFactoryTest, StartFailedOpen) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -182,8 +182,8 @@ TEST_P(WasmFactoryTest, StartFailedOpen) {
}

TEST_P(WasmFactoryTest, ConfigureFailed) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand Down
4 changes: 2 additions & 2 deletions test/extensions/bootstrap/wasm/wasm_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ INSTANTIATE_TEST_SUITE_P(Runtimes, WasmIntegrationTest,
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WasmIntegrationTest);

TEST_P(WasmIntegrationTest, FilterMakesCallInConfigureTime) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand Down
32 changes: 16 additions & 16 deletions test/extensions/bootstrap/wasm/wasm_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ TEST_P(WasmTestMatrix, LoggingWithEnvVars) {
}

TEST_P(WasmTest, BadSignature) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -186,8 +186,8 @@ TEST_P(WasmTest, BadSignature) {
}

TEST_P(WasmTest, Segv) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -205,8 +205,8 @@ TEST_P(WasmTest, Segv) {
}

TEST_P(WasmTest, DivByZero) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -224,8 +224,8 @@ TEST_P(WasmTest, DivByZero) {
}

TEST_P(WasmTest, IntrinsicGlobals) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -249,8 +249,8 @@ TEST_P(WasmTest, IntrinsicGlobals) {
// change this behavior by providing non-trapping instructions, but in the mean time we support the
// default Emscripten behavior.
TEST_P(WasmTest, Asm2Wasm) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -267,8 +267,8 @@ TEST_P(WasmTest, Asm2Wasm) {
}

TEST_P(WasmNullTest, Stats) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand All @@ -292,8 +292,8 @@ TEST_P(WasmNullTest, Stats) {
}

TEST_P(WasmNullTest, StatsHigherLevel) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand Down Expand Up @@ -323,8 +323,8 @@ TEST_P(WasmNullTest, StatsHigherLevel) {
}

TEST_P(WasmNullTest, StatsHighLevel) {
#if defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if !defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
if (GetParam() != "null") {
return;
}
Expand Down
4 changes: 2 additions & 2 deletions test/extensions/common/wasm/wasm_runtime.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ std::vector<std::string> sandboxRuntimes() {

std::vector<std::string> languages() {
std::vector<std::string> languages;
#if !defined(__aarch64__)
// TODO(PiotrSikora): There are no Emscripten releases for arm64.
#if defined(__x86_64__)
// TODO(PiotrSikora): Emscripten ships binaries only for x86_64.
languages.push_back("cpp");
#endif
languages.push_back("rust");
Expand Down
Loading