Add tag-based test filtering by mingnus · Pull Request #8 · device-mapper-utils/dmtest-python

mingnus · 2026-06-29T19:25:31Z

Add a tagging system for test categorization and filtering. Tests can carry tags like smoke, benchmark, or experimental to classify their purpose beyond the existing path-based organization. A boolean expression parser lets users select tests by tags at the command line or through a config default.

Motivation

The test suite contains tests of varying nature: parameter validation, smoke tests, IO-intensive functional tests, benchmarks, and stress tests. However, they are organized along a single axis (by feature, subsystem, or module). Inspired by pytest markers, tags add a second axis so developers can run only fast deterministic tests during development, or exclude benchmarks in CI, without reorganizing the test tree.

Changes

Config restructuring: Split the flat config.toml into [devices] and [run] sections with a new Config class that centralizes key definitions and defaults.
Tag registration: Tests can carry tags via the registration tuple. register_batch() supports per-test and batch-level tags, merged at registration time.
Tag expression parser: A parser compiles boolean expressions like "!experimental" or "smoke & !benchmark" into matcher objects. Supports boolean operators (&, |, !), parentheses, and keyword aliases (and, or, not).
--tags filtering: A new --tags CLI option filters tests by tag expression, AND'd with existing path/state filters. A default expression can be set in config.toml under [filter].tags; the CLI option overrides it.
Tag display: list -T shows each test's tags, and list-tags subcommand summarizes all tags with test counts.

Registration examples

# Per-test tags in the 3rd element (list)
tests.register_batch(
    "/thin/creation/",
    [   
        ("lots-of-empty-thins", t_lots_of_empty_thins, ["smoke", "stress"]),
        ("huge-block-size", t_huge_block_size, ["smoke"]),
        ("too-large-block-size", t_too_large_block_size, ["validation"]),
    ],  
)

# Per-test tags with dep_fn in the 4th element
tests.register_batch(
    "/thin/creation/",
    [   
        ("snapshot-create", t_snapshot_create, dep_fn, ["functional"]),
    ],  
)

# Batch-level tags applied to all tests in the group
tests.register_batch(
    "/thin/benchmarks/",
    [   
        ("provisioning", t_provisioning),
        ("dd-performance", t_dd_performance),
    ],  
    batch_tags=["benchmark"],
)

Usage

# Run only smoke tests
dmtest run --tags smoke

# Exclude benchmarks and experimental tests
dmtest run --tags '!(benchmark | experimental)'

# Disable the default tag filter from config.toml
dmtest run --tags ""

# List tests with tags shown
dmtest list -T

# See all defined tags
dmtest list-tags

Restructure existing keys in config.toml into sections for better extensibility, and introduce a Config class that centralizes config key definitions, section routing, and default value lookup, replacing duplicated defaults across tests. Now a [devices] section is required and the old flat format is rejected.

Factor out the tuple parsing logic to support tags in later commits

Tests can now carry tags by appending a list to the registration tuple: - 3-tuple (tags only): ("test-name", callback, ["tag1", "tag2"]) - 4-tuple (dep_fn + tags): ("test-name", callback, dep_fn, ["tag1"]) Tags are stored as a frozenset in the Test definition, defaulting to empty for backward compatibility.

Implement a recursive descent parser that compiles tag expressions like "!(benchmark | experimental)" into a matcher object. The matcher takes a list of tags and returns whether they satisfy the expression. The parser supports boolean operators (&, |, !) and parentheses for grouping. Identifiers consist of alphanumeric characters and underscores, and must not start with a digit.

Pass the full Test struct to matches() to enable tag-based filtering in later commits.

The CLI --tags option accepts a boolean tag expression to filter tests by their tags. A tag filter built from the expression is AND'd with existing path/state filters, narrowing the selected tests. A default expression can be set in config.toml under [filter].tags. The --tags option overrides it completely.

Show each test's tags on the list

lorelei-sakai

Generally speaking this looks fine. Most of my comments are about parts of the implmentation that seem awkward or over-complicated.

We should communicate to existing users that the current config file format will break once you commit this and people will have to reshape their config files; it could lead to a bit of surprise otherwise.

lorelei-sakai · 2026-06-30T16:50:29Z

+        if cfg.get(key) is None:
+            raise ValueError(f"config.toml: missing required key '{key}' in [devices]")
+    if not cfg.get("disable_by_id_check"):
+        check_dev(cfg.get("metadata_dev"), "metadata_dev")


check_dev here really doesn't operation on anything except the config, so it probably makes more sense as a function in the Config class.
Then you could call it simply as cfg.check_dev("data_dev") with fewer arguments.

Possibly the entire validate() function should also be defined in the Config class as well, though that makes less difference in terms of code complexity.

Good point, I'll do that.

lorelei-sakai · 2026-06-30T16:52:57Z

-    def __str__(self):
-        return str(self._cfg)
-
    def __init__(self):


Now that you have an actual Config class, the Fixture class (which seems to just be a holder for the config) doesn't seem to serve much purpose. I'd like to get rid of one of the other and only have one class.

That can be a future cleanup, though.

That makes sense. My thinking was that Fixture will likely grow to carry other environmental information beyond just configs. However, I agree we can revisit this for a cleanup later.

lorelei-sakai · 2026-06-30T18:05:48Z

-    raise ValueError(f"test entry must have 2-3 elements, got {len(entry)}")
+        path, callback, third = entry
+        if callable(third):
+            return path, callback, third, None


This parsing dance seems really awkward. Did you consider using named parameters instead of trying to guess what the third argument is by type?

In particular, I think we could make the third argument always "tags" and then require a named argument for tests that want to set dep_fn. There are currently only three tests that register a dep_fn, and it's for enforcing a dependency that I hope to remove later anyway, so in the long run we can probably make dep_fn go away entirely.

Tuples don't have named parameters. Switching to dicts introduces more verbosity, and using NamedTuple requires a class name on every test definition. Both introduce a lot of churn.

Making the third argument always "tags" could be a feasible direction since only three tests need a dep_fn. Those three tests could be registered separately via register() directly.

lorelei-sakai · 2026-06-30T18:07:46Z

        self._tests = {}

-    def register(self, path, callback, dep_fn=None):
+    def register(self, path, callback, dep_fn=None, tags=None):


If you make tags default to an empty list, I think it simplifies the assignment later. You should then be able to always say t = frozenset(tags) or even inline it.

Use empty iterable default to avoid if-else:

def register(self, path, callback, dep_fn=None, tags=()): path = _normalise_path(path) self._tests[path] = Test(dep_fn, callback, frozenset(tags))

The type check on tags and dep_fn could be moved from _parse_test_entry() to register()

lorelei-sakai · 2026-06-30T18:15:52Z

        self._tests[path] = Test(dep_fn, callback, t)

-    def register_batch(self, prefix, tests, batch_dep_fn=None):
+    def register_batch(self, prefix, tests, batch_dep_fn=None, batch_tags=None):


I think the case is even stronger here for making batch_tags the third argument and requiring a named-parameter to set the batch_dep_fn. There is literally no current test that sets a batch_dep_fn so you won't even have to update anything.

Agree. Moving batch_tags to the third positional argument makes it more convenient for callers:

def register_batch(self, prefix, tests, batch_tags=(), batch_dep_fn=None):

lorelei-sakai · 2026-06-30T19:38:55Z

@@ -4,31 +4,31 @@



The plit between patches 7 and 8 feels awkward ("Extend TestFilter to obtain Test properties in matches()" and "Add tag-based test filtering via --tags option"). It's not at all clear in patch 7 why you want to pass all the test properties down through all the filters. Maybe pull the TagFilter class into the earlier patch?

This probably doesn't matter than much, feel free to ignore it.

I had similar thoughts, but keeping the refactor separate makes the diff easier to review since it's a pure interface change across many files. The separation also makes it clear that the mechanical update is correct on its own, independent of the feature addition.

I'll update the commit message to explain why the interface is changing:

Extend TestFilter.matches() to accept the Test struct Pass the full Test struct alongside the path so that filters can access test properties like tags. This is needed for the TagFilter introduced in the next commit.

The extra info in the commit message seems like enough, thanks.

lorelei-sakai · 2026-06-30T20:39:45Z

+        if args.show_tags:
+            tags = tests.get_tags(p)
+            tag_str = f"[{', '.join(sorted(tags))}]" if tags else ""
+            print(f"{tag_str.ljust(30)}", end=" ")


It's not a big deal, probably but, I'm concerned about what this formatting will look like. tree_line() justifies everything so the test name will take up 50 columns. You're adding another 30 columns, so the results will necessarily show up past column 80 here, which is awkward if you're using an 80-character-wide terminal (like I do).
Another concern is that if there are three or four tags, they may take more than 30 columns to print.
Perhaps you could steal some columns from tree_line? Perhaps you could put the tags after the results columns? Perhaps you could deliberately add a second line for the tags of each test?

(Also tags here is guaranteed to be a frozenset I think, you shouldn't need the if/else.)

lorelei-sakai · 2026-06-30T20:40:36Z

    arg_result_set(list_p)
    arg_run_nr(list_p)
+    list_p.add_argument(
+        "-T",


Is it worth adding a long form of this argument too, like --show-tags?

I initially chose -T since the verb-prefixed --show-tags felt redundant to the list subcommand, but I could add both for discoverability.

The verb is kind of redundant, yeah. The longer tag could be just --tags? It just feels weird to only have the short option when most other command arguments don't do that.

--tags is used for specifying tag expression and the list subcommand supports --tags, so the name is taken, unless we rename the --tags option. I haven't figured out a good long option name for -T.

pytest's -m option (marker expression) doesn't have the correspond long option name.

raeburn · 2026-06-30T23:07:48Z

+        if token is None:
+            raise ParseError("unexpected end of expression, expected an identifier or '('")
+        if token in ('&', '|', ')'):
+            raise ParseError(f"unexpected '{token}', expected an identifier or '('")


It looks to me like if "and" shows up in the wrong place, it'll be reported as "unexpected '&'" won't it? (Similarly for "or".)

Right, keywords "and", "or", "not" are replaced. The error message could be ambiguous.

raeburn · 2026-06-30T23:40:58Z

+        self.name = name
+
+    def match(self, tags):
+        return self.name in tags


So, I take it the tag names accepted are all arbitrary, not limited to a set actually used in the tree? If I type "smoek" instead of "smoke" I just don't get any tests run, no warning/error? (Worse, "tag1|tag2" when I spell one wrong would still run the tests with the other tag, making it less obvious that something was wrong.)

mingnus added 10 commits June 30, 2026 02:30

Fix argument passing in dmtest wrapper script

1663f50

Extract _parse_test_entry() from register_batch()

99999e8

Factor out the tuple parsing logic to support tags in later commits

Support batched tags in register_batch()

eea288b

Extend TestFilter to obtain Test properties in matches()

6f820f0

Pass the full Test struct to matches() to enable tag-based filtering in later commits.

Add -T flag to list subcommand for tag display

e038c8b

Show each test's tags on the list

Add list-tags subcommand to show all tags and their test counts

bd21fdc

lorelei-sakai requested changes Jun 30, 2026

View reviewed changes

raeburn requested changes Jun 30, 2026

View reviewed changes

Uh oh!

Conversation

mingnus commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Registration examples

Usage

Uh oh!

lorelei-sakai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mingnus Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mingnus Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mingnus Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mingnus Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mingnus commented Jun 29, 2026 •

edited

Loading

mingnus Jul 1, 2026 •

edited

Loading

mingnus Jul 1, 2026 •

edited

Loading

mingnus Jul 2, 2026 •

edited

Loading

mingnus Jul 1, 2026 •

edited

Loading