Is https://sites.google.com/a/chromium.org/dev/chromium-os/testing/dynamic-test-suites, and specifically the attr_filter argument in autotest's site_utils/run_suite.py, where I should be looking?

Should I add a new optional attribute to suite control files that can be used to specify a boolean expression of attributes that will be used in place of a suite-name-based expression if it's set? ATTRIBUTE_EXPRESSION or something like that?

RE 1 that approach sounds fine.

The suite control files themselves contain the call to dynamic_suite that actually creates the suite. This call passes in a bunch of arguments. The control files that use ATTRIBUTE_EXPRESSION can just pass ATTRIBUTE_EXPRESSION directly in as the appropriate argument to dynamic_suite.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/48276a0e48120b9dbb6305ccbb478f681fb54698

commit 48276a0e48120b9dbb6305ccbb478f681fb54698
Author: Daniel Erat <derat@chromium.org>
Date: Fri May 26 16:47:44 2017

autotest: Clean up dynamic_suite.py.

Remove obsolete documentation (the arguments to SuiteSpec's
__init__() method were documented in three places; they
didn't match, of course). Also update docs to match argument
order.

BUG=chromium:714865
TEST=dynamic_suite_unittest.py passes

Change-Id: Icc215e884c9f02337f95e6217aa4408cd37ef752
Reviewed-on: https://chromium-review.googlesource.com/515602
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/48276a0e48120b9dbb6305ccbb478f681fb54698/server/cros/dynamic_suite/dynamic_suite.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/a26332f4e0a1a15508a81265fb0e3b8ada44c95b

commit a26332f4e0a1a15508a81265fb0e3b8ada44c95b
Author: Daniel Erat <derat@chromium.org>
Date: Fri May 26 16:47:44 2017

autotest: Remove unused dynamic_suite timeout arg.

Remove dynamic_suite.SuiteSpec's timeout arg, which has been
deprecated in favor of timeout_mins. Also remove an unused
DEFAULT_TRY_JOB_TIMEOUT_MINS constant.

BUG=chromium:714865
TEST=dynamic_suite_unittest.py passes

Change-Id: I0709b5c17a323cf2edf9f2b7095d929f4313e03d
Reviewed-on: https://chromium-review.googlesource.com/516202
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/a26332f4e0a1a15508a81265fb0e3b8ada44c95b/server/cros/dynamic_suite/dynamic_suite.py

I'm having a difficult time getting a big-picture view of the code that runs tests.

When I look at a suite control file like test_suites/control.bvt-inline, it ends with a call to dynamic_suite.reimage_and_run(). I have an in-progress change that updates dynamic_suite.SuiteSpec to support creating predicates consisting of boolean expressions of attributes. We already have Suite.matches_attribute_expression_predicate, so this ended up being a small change.

However, when I run a test suite locally using "test_that ... suite:foo", it looks like we skip all of that and just compose a list of tests that have "suite:foo" attributes using Suite.name_in_tag_predicate. Is my interpretation correct? If so, why do we have two disjoint code paths for getting the tests belonging to a given suite? Was this an intentional design choice?

If we switch over to expression-derived test suites, I'm assuming that we'll want to be able to run them locally in addition to on builders. Do I need to change site_utils/test_runner_utils.py to always parse suites' control files and construct SuiteSpec objects? How would I even go about doing that? It feels like I'd need to move the attribute expression into a top-level variable instead of passing it to dynamic_suite.reimage_and_run(), which is called at the bottom of suite control files but which we presumably wouldn't want to use for local runs.

Per VC discussion with Aviv today:

- Attribute expressions should be (optionally) declared in a top-level variable, e.g. ATTRIBUTE_EXPRESSION, in suite control files.
- test_that should be changed to parse suite control files and use ATTRIBUTE_EXPRESSION if present or its current suite-name-in-attribute predicate otherwise.
- "test_that ... :lab: ... suite:..." should probably be changed to exit with an error telling the user to use run_suite.py instead. (I don't understand test_that and run_suite.py enough to know why, but otherwise, the wrong tests may be run in some cases where there's a version mismatch, I think.)

I'm not immediately sure whether there will be any way for dynamic_suite.reimage_and_run() to access the top-level ATTRIBUTE_EXPRESSION variables or if they'll additionally need to be passed as an arg. I find the control file parsing code hard to follow due to "magic" behavior where (if I was reading it correctly) it automatically lowercases control file variables and converts them into properties.

I'm not going to get to this. I uploaded my in-progress change at https://crrev.com/c/517362. I think that it probably works in the lab, but I completely missed that test_that has its own code path for running suites that (unsurprisingly) skips the dynamic_suite.reimage_and_run call in the suite control files.

(It feels wrong to me that control files contain both declarative configuration and function calls that reimage devices and execute tests.)

Hi, this bug has not been updated recently. Please acknowledge the bug and provide status within two weeks (6/22/2018), or the bug will be archived. Thank you.