//src/test/java/com/google/devtools/build/lib/rules/cpp:cpp-rules-tests not running on Windows

[meteorcloudy]

I just noticed cpp-rules-tests is not running on Windows for a while.
I believe we missed many Windows breakage due to this.

I believe this is due to our change for Windows tests configuration?
See

bazel/src/test/java/com/google/devtools/build/lib/BUILD

Line 1361 in 275ae45

name = "all_windows_tests",

@laszlocsomor Any specific reason we are not running this test?

[laszlocsomor]

I remember checking that //:all_windows_tests contained the same tests as the query used to in bazel-tests.json.

Indeed, commit 3f2f480 moved //src/test/java/com/google/devtools/build/lib:cpp-rules-tests to //src/test/java/com/google/devtools/build/lib/rules/cpp:cpp-rules-tests about 2 weeks ago, and I suppose @c-parsons didn't know he had to update the all_windows_tests rules.

Yun, do you have any idea how we could prevent such inadvertent test suite changes in the future?

[meteorcloudy]

I see, thanks for looking into it.
Did we refer cpp-rules-tests explicitly?
Because if we did, moving it should cause an error, right?

[laszlocsomor]

Did we refer cpp-rules-tests explicitly?
Because if we did, moving it should cause an error, right?

We didn't. It was matched by the test_suite in its old package:

bazel/src/test/java/com/google/devtools/build/lib/BUILD

Lines 1351 to 1358 in d0bf589

	test_suite(
	name = "windows_tests",
	tags = [
	"-no_windows",
	"-slow",
	],
	visibility = ["//visibility:private"],
	)

[meteorcloudy]

I see, explicitly refer every test is not feasible. But can test_suite also match tests under its subpackage?

[meteorcloudy]

Or we have to use genquery to check the tests lists match every time..

[laszlocsomor]

No, test_suite cannot match tests in subpackages.
I'm not aware of any better solution than using genquery and comparing its output to a golden file.

[meteorcloudy]

Or comparing its output to another genquery? Something like:
//src/test/java/... //src/test/cpp/... //src/test/py/... --tag=-no_windows

So that we don't have to modify the test list every time it changes.

[laszlocsomor]

Sure, if genquery supports "..." targets then let's do that.

[meteorcloudy]

Yes, it does.

pcloudy@pcloudy0-w MSYS ~/workspace/bazel
$ bazel-0.7.0 query 'tests(//src/test/py/...)'
...........
//src/test/py/bazel:launcher_test
//src/test/py/bazel:bazel_windows_test
//src/test/py/bazel:bazel_windows_cpp_test
//src/test/py/bazel:bazel_server_mode_test
//src/test/py/bazel:bazel_external_repository_test
//src/test/py/bazel:bazel_clean_test
//src/test/py/bazel:action_temp_test

[laszlocsomor]

"bazel query" and "genquery" are slightly different:

The only difference between the queries allowed here and on the command line is that queries containing wildcard target specifications (e.g. //pkg:* or //pkg:all) are not allowed here. The reasons for this are two-fold: first, because genquery has to specify a scope to prevent targets outside the transitive closure of the query to influence its output; and, second, because BUILD files do not support wildcard dependencies (e.g. deps=["//a/..."] is not allowed).

(source: https://docs.bazel.build/versions/master/be/general.html#genquery)

And unfortunately in this case they are different.

Query:

d:\os\bazel>bazel query "kind(filegroup, '//foo/...')"
//foo/bar:srcs
//foo:srcs

Genquery:

d:\os\bazel>bazel query --output=build //foo:q
# D:/os/bazel/foo/BUILD:7:1
genquery(
  name = "q",
  scope = ["//foo:__subpackages__"],
  expression = "kind(filegroup, '//foo/...')",
)


d:\os\bazel>bazel build //foo:q
ERROR: D:/os/bazel/foo/BUILD:7:1: in genquery rule //foo:q: query failed: recursive target patterns are not permitted: '//foo/...''.
ERROR: Analysis of target '//foo:q' failed; build aborted: Analysis of target '//foo:q' failed; build aborted.
INFO: Elapsed time: 0.435s

[meteorcloudy]

;( Looks like the golden test list is the only way ...

[laszlocsomor]

This bug is superseded by #4292. Dropping to P2 to match that bug's current priority.

[laszlocsomor]

aaand raising to P1 to match #4292 's new priority.