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.

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 test_suite for dbx_services_test #25

Closed
olschofkah opened this issue Jan 21, 2021 · 6 comments
Closed

Support test_suite for dbx_services_test #25

olschofkah opened this issue Jan 21, 2021 · 6 comments

Comments

@olschofkah
Copy link

Would it be possible to add support for test suites for the 'test' invoked on a dbx_services_test? The use case is that a collection of tests that can run in parallel may share a set of defined daemon or task services that need to be performed. Right now the following error occurs:

Traceback (most recent call last): File "/root/.cache/bazel/_bazel_root/7ebbdddc3c30692c56fb633c56be95c3/external/dbx_build_tools/build_tools/services/svc.bzl", line 481, column 75, in services_bin_impl launcher_args.append("--svc.test-bin={}".format(ctx.executable.bin.short_path)) Error: 'NoneType' value has no field or method 'short_path'

@benjaminp
Copy link
Contributor

Traceback (most recent call last): File "/root/.cache/bazel/_bazel_root/7ebbdddc3c30692c56fb633c56be95c3/external/dbx_build_tools/build_tools/services/svc.bzl", line 481, column 75, in services_bin_impl launcher_args.append("--svc.test-bin={}".format(ctx.executable.bin.short_path)) Error: 'NoneType' value has no field or method 'short_path'

What are you doing to cause this error?

@olschofkah
Copy link
Author

You'll see this error if you create a bazel test suite to wrap the test you're intending to run, and then pass that into the test field of dbx_services_test. Here's some pseudo code

py_test(
  name = "a-test-1",
  srcs = "//pycode:test.py",
  main = "//pycode:test.py",
)

py_test(
  name = "a-test-2",
  srcs = "//pycode:test2.py",
  main = "//pycode:test2.py",
)

test_suite(
  name = "the-test-suite",
  tests = [
    ":a-test-1",
    ":a-test-2",
  ],
)

dbx_services_test(
  name = "the-test-suite-with-shared-runtime-services",
  args = [...], # note here that the args are passed to the test and override args test itself ... which doesn't exactly work for a test suite concept as the args may be different per test. 
  test = ":the-test-suite",
  services = [...],
)

@benjaminp
Copy link
Contributor

This isn't possible to support today because test_suite is intended only as a top-level rule by Bazel. For example, to do this, we'd have to be able to see the individual test targets in the test_suite, but test-suite just doesn't expose that.

@olschofkah
Copy link
Author

olschofkah commented Jan 23, 2021

I see. The test suite itself doesn't actually provide much value and only acts as a grouping of tests mechanic. The same problem could be solved by just allowing a collection of tests to be passed directly to dbx_services_test if it's only a problem of visibility.

dbx_services_test(
  name = "the-tests-with-shared-runtime-services",
  args = [...], 
  tests = [
    ":a-test-1",
    ":a-test-2",
  ],
  services = [...],
)

You'd likely not want to have an empty set of args override args set on the individual tests in this case. I'm not sure you really want to be doing that anyway? I found that a bit confusing.

Just as an fyi, I have tried the other direction ( putting multiple dbx_services_test in a test suite). It works as you'd expect with each of the services performing their work independently. There could be an approach here where for services, your DAG of services would reduce down to a single execution of the daemon/task. There are many reasons why you may not be able to concurrently run the same daemon/task in a single bazel execution ... such as ports being bound, cloud asset changes, non-sandbox file system changes, etc ... and running them sequentially may be expensive (it is in my case). I'd still assume you'd want to be able to opt into the current behavior though in the case of a test that mutates the state of the daemon running.

@benjaminp
Copy link
Contributor

I see. The test suite itself doesn't actually provide much value and only acts as a grouping of tests mechanic. The same problem could be solved by just allowing a collection of tests to be passed directly to dbx_services_test if it's only a problem of visibility.

dbx_services_test(
  name = "the-tests-with-shared-runtime-services",
  args = [...], 
  tests = [
    ":a-test-1",
    ":a-test-2",
  ],
  services = [...],
)

Sure. Ultimately, this is going to boil down to running many executables within one Bazel test target. That's already possible today with a wrapper that invokes the individual tests, maybe with another rule to wrap it nicely.

Just as an fyi, I have tried the other direction ( putting multiple dbx_services_test in a test suite). It works as you'd expect with each of the services performing their work independently. There could be an approach here where for services, your DAG of services would reduce down to a single execution of the daemon/task. There are many reasons why you may not be able to concurrently run the same daemon/task in a single bazel execution ... such as ports being bound, cloud asset changes, non-sandbox file system changes, etc ... and running them sequentially may be expensive (it is in my case). I'd still assume you'd want to be able to opt into the current behavior though in the case of a test that mutates the state of the daemon running.

We run our tests in Linux mount and network namespaces, so these kinds of conflicts are not a problem with concurrent executions. And there are strong motivations to avoid sharing between tests such as undebuggable isolation violations.

@jhance
Copy link
Contributor

jhance commented Sep 27, 2021

Mis-understanding of how test_suite works, so closing.

@jhance jhance closed this as completed Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants