[PATCH] D119547: [libc][bazel] Add tests to the bazel build

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added a subscriber: lntue.
gchatelet added inline comments.


================
Comment at: utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl:16
+    if "deps" in kw:
+        fail("A libc_test target should list 'libc_function_deps' and " +
+             "'additional_deps' instead of 'deps'.")
----------------
GMNGeoffrey wrote:
> It seems like calling the "additional_deps" argument "deps" would also be fine? Generally it's preferred to use standard names (https://bazel.build/rules/bzl-style). I do see the argument for different naming, thouhg. Your call
@sivachandra WDYT?


================
Comment at: utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl:36
+        additional_deps = additional_deps,
+        size = "enormous",  # Some tests are emulated and take very long.
+    )
----------------
GMNGeoffrey wrote:
> Oh no 😨 How much does this extend CI runtime? Also, how many tests are like this? Instead of hardcoding "enormous" for every test, could you just pass that argument for the few that are in fact enormous? Separately, I think it is a better fit to use the `timeout` argument here `timeout = "eternal"`. Test size is ill-defined, but I think by any reasonable definition this test isn't an enormous one. Those usually involve spinning up whole servers and such. I think it's a small test that just happens to take a very long time.
It is not //that// bad. Here are the top tests sorted by runtime:
```
@llvm-project//libc/test/src/math:hypot_test                             PASSED in 44.1s
@llvm-project//libc/test/src/math:hypotf_test                            PASSED in 31.4s
@llvm-project//libc/test/src/math:remquol_test                           PASSED in 19.7s
@llvm-project//libc/test/src/math:sincosf_test                           PASSED in 12.8s
@llvm-project//libc/test/src/math:sqrtf_test                             PASSED in 6.3s
@llvm-project//libc/test/src/math:sqrt_test                              PASSED in 6.0s
@llvm-project//libc/test/src/math:sqrtl_test                             PASSED in 4.8s
@llvm-project//libc/test/src/math:sinf_test                              PASSED in 3.8s
...
```

@lntue can we add the size tag only on the few bad ones?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119547/new/

https://reviews.llvm.org/D119547