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

Add a way to reset or remove from a bundle by rule #3792

Closed
kkpattern opened this issue Nov 16, 2023 · 1 comment
Closed

Add a way to reset or remove from a bundle by rule #3792

kkpattern opened this issue Nov 16, 2023 · 1 comment

Comments

@kkpattern
Copy link

Thanks so much for this awesome tool! Recently, when we test with stateful testing, we found that we really wanted a way to reset or remove some data from a bundle in a rule.

Specifically, we were testing a tree-based algorithm. We had several rules to add and remove nodes from a tree. Like this:

class TreeDiffState(RuleBasedStateMachine):
    node_keys: Bundle = Bundle("node_keys")

    @rule(target=node_keys)
    def add_node(self, k):
        ...

    @rule(k=consumes(node_keys))
    def remove_node(self, k):
        ...

The problem was that when a node was removed from a tree, not only the node itself was removed, but all its children were removed too.

Given a tree like this:

Node 1
    |-- Node 2
            |-- Node 3
    |-- Node 4

The corresponding node_keys bundle contains the following items: (1, 2, 3, 4).

When 2 was drawn from node_key to remove, the tree became like this:

Node 1
    |-- Node 4

The corresponding node_keys should be (1, 4) but actually (1, 3, 4).

Then, in the following steps, 3 may still be drawn. This would cause two problems:

  1. If we got a key that was not in the tree, we didn't know if it was a bug in our code
  2. Hypothesis may generate unnecessary steps and cause inefficient test

Currently, we can work around this by setting the bundle value directly like this:

    @rule(k=consumes(node_keys))
    def remove_key(self, k):
        self._tree.remove_node(k)
        self.bundles["node_keys"] = list(self._tree.nodes.keys())

But this is not a public API. In the long run, having a public API to reset or remove data from a bundle would be better. Maybe something like:

    @rule(k=consumes(node_keys), reset_target=node_keys)
    def remove_key(self, k):
        self._tree.remove_node(k)
        return list(self._tree.nodes.keys())

If the design is OK, we are happy to help prepare a PR.

Thanks.

@Zac-HD
Copy link
Member

Zac-HD commented Nov 16, 2023

I'm concerned that this proposed API will be difficult to use correctly. Could you solve your problem by storing entire trees in the bundle, instead of keys, and returning a new tree?

I think it's already technically feasible to store the list of live node keys as an instance variable, draw keys from a sampled_from() defined inside your test method, and modify the list before continuing. Definitely less elegant, though.

@Zac-HD Zac-HD closed this as not planned Won't fix, can't repro, duplicate, stale Nov 27, 2023
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

2 participants