Windows: bring back tests disabled by #13133

[sunjayBhatia]

Commit Message: Windows: bring back tests disabled by #13133
Additional Description:
PR #13133 disabled various tests as they failed or flaked when compiled
with clang-cl. Given we have some significant other blockers to adding
clang-cl support and enabling as many tests to run in CI as possible is
important to progress to beta/GA support, we can un-skip these tests.

We could be "clever" and use a select to only tag tests to be skipped
when built with a specific compiler and compilation mode as well as an
alternative.

Risk Level: Low
Testing: Re-enables tests in the Windows pipeline
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

[sunjayBhatia]

cc @envoyproxy/windows-dev would you prefer to take this PR as-is or add some select logic to the tagging, e.g.

tags = [] + select({
  "//bazel:clang_cl_build": ["fails_on_windows"],
  "//conditions:default": [],
}),

[davinci26]

if it is just as easy as adding the following tag:

tags = [] + select({
  "//bazel:clang_cl_build": ["fails_on_windows"],
  "//conditions:default": [],
}),

I would just add it so it's easier for someone to contribute and help out with the clang-cl failures. Alternatively we could have a help wanted issue and write down the failures there.

[sunjayBhatia]

One thing we realize, is you won't be able to do a bazel query for the known failing/flaky/not compiling tests unless you have the correct config enabled (e.g. bazel query --config=clang-cl ...) so it maybe makes finding things harder on non-Windows/without toolchains configured

[sunjayBhatia]

Also the select logic is somewhat less discoverable than just opening issues (though we could do both) as I doubt people who are not in the know will dig through the BUILD files for things to do

[sunjayBhatia]

/retest

clang tidy took too long

[repokitteh-read-only]

Retrying Azure Pipelines.

🐱

Caused by: a #13711 (comment) was created by @sunjayBhatia.

see: more, trace.

[wrowe]

My thought on this, I'd rather avoid windows devs tripping over these tests during the adoption phase of clang-cl, even though this is not yet enabled on the CI pipeline, and for them to be aware these need fixing before we can enable them on that not-so-far-future pipeline. Guessing their resolution is fairly straightforward, but spending cycles on true msvc-cl failures seems like the top priority in the immediate-term.

[sunjayBhatia]

/retest

[repokitteh-read-only]

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13711 (comment) was created by @sunjayBhatia.

see: more, trace.

[wrowe]

Alright then, let's merge this to master, then revert this from the clang-cl enablement PR #13688 with the proposed select() decoration. We should be able to be more specific than clang_cl and specify even where the test fails only under an opt, dbg or fastbuild construction.

[sunjayBhatia]

if it is just as easy as adding the following tag:

tags = [] + select({
  "//bazel:clang_cl_build": ["fails_on_windows"],
  "//conditions:default": [],
}),

I would just add it so it's easier for someone to contribute and help out with the clang-cl failures. Alternatively we could have a help wanted issue and write down the failures there.

see #13711 (review) we're gonna do the select logic in the clang-cl toolchain enabling PR so it's all focused since they are all clang-cl issues

[davinci26]

@envoyproxy/senior-maintainers this PR is all good from windows side.