Skip to content

Fix test prompt filtering - #290

Open
olivercalder wants to merge 12 commits into
canonical:mainfrom
olivercalder:fix-test-prompt-filtering
Open

Fix test prompt filtering#290
olivercalder wants to merge 12 commits into
canonical:mainfrom
olivercalder:fix-test-prompt-filtering

Conversation

@olivercalder

Copy link
Copy Markdown
Member

It seems the top-level prompt filters in scripted client sequence files were being ignored. In particular, these top-level filters are supposed to filter out which prompts should be ignored by the test rather than being matched against the expected sequence of prompts.

The PromptSequence field name for the filter has simply been filter, while the documentation and sequence files use "prompt-filter" for the top-level prompt filter as well as the prompt matching within the sequence. As a result, the prompt sequence creation never deserialized the top-level filters correctly, leaving them as None.

Rename the field to prompt_filter so it is deserializes from "prompt-filter" and matches the documentation and test files.

Additionally, various test cases and files had broken top-level filters due to the fact that none were being used or tested. Fix them, add top-level filters to the remaining tests which lacked them, and add a dedicated test that the top-level filters work by running two test snaps in parallel in different directories.

Lastly, to avoid any conflicts between test cases when running the tests which simply use the expect_single_prompt macro, add a filter to that macro which filters out prompts for the given interface and expects a parent directory when when run with the home interface, which it uses as a prefix filter. Since tests are run in serial, this may be unnecessary, but I believe some tests do (currently) leave behind running test snaps which can trigger prompts across test cases.

The `PromptSequence` field name for the filter has simply been `filter`,
while the documentation and sequence files use `"prompt-filter"` for the
top-level prompt filter as well as the prompt matching within the
sequence.

Rename the field to `prompt_filter` so it is serialized to
`"prompt-filter"` and matches the documentation and test files.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Also, ensure the `BASE_PATH` variable mapping is correctly set for all
test cases which use it.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
@olivercalder

Copy link
Copy Markdown
Member Author

With the top-level prompt filter now in use, every test which uses a scripted client hangs. This suggests the filter isn't matching the expected prompts. I did check the source code and the matching code for prompt filters looks reasonable, with nothing obviously wrong. Notably, if a prompt filter has some fields absent, those fields are ignored and do not cause a match to fail.

Also, I think some tests leave behind active test snap instances running in the background, which I think could be making some tests hang (though some tests where this ought not to be the case, e.g. scripted_client_works_with_simple_matching, still hang even if run in isolation -- or at least if all the other tests which use scripted clients are marked as #[ignore]). For example, I think invalid_prompt_sequence_reply_errors leaves aa-prompting-test.create running until it times out, which may be longer than the test timeout.

Interestingly, when the tests hang, they don't automatically fail after the new TIMEOUT, though that's not particularly surprising, as the timeout is specific to integration.rs and does not appear in the scripted client code where the waiting must be occurring.

@codecov

codecov Bot commented Dec 17, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.96%. Comparing base (f15183c) to head (57757c3).
⚠️ Report is 107 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #290   +/-   ##
=======================================
  Coverage   80.96%   80.96%           
=======================================
  Files          21       21           
  Lines         725      725           
=======================================
  Hits          587      587           
  Misses        138      138           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gabelluardo
gabelluardo self-requested a review December 17, 2025 10:01
…ible

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
@olivercalder

Copy link
Copy Markdown
Member Author

I didn't close this???

@olivercalder olivercalder reopened this Dec 18, 2025
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
For `TypedPromptFilter` and `TypedPrompt`, use the `"interface"` field
as the deserialization tag, and remove that field from the inner generic
types which get associated directly with the interface type.

It is nonsensical to have an interface field with a value which differs
from the interface type associated with its containing struct.

Importantly, this allows these typed structs to be deserialized to the
correct interface. Previously, each interface was tried in order, and
whichever succeeded first was used. This resulted in `TypedPromptFilter`
instances always being deserialized to the `Camera` variant, which
caused the filter to fail to match any prompt for another interface.

Notably, this means that the `"interface"` field is now required when
deserializing `TypedPromptFilter` and `TypedPrompt` values. It will
always be present for prompts, and it makes sense to require it for
prompt filters as well in order to parse the other fields correctly.

Lastly, add lots of tests related to filters deserialization and
matching.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
If a scripted client sequence ends with a grace period, continue to obey
the prompt filter when watching for any unexpected prompts.

Previously, the prompt filter was ignored when watching for prompts
during the grace period, meaning that any leftover prompt from any other
test would be caught during the grace period and cause the test to fail,
as well as interfere with that other test since the matching prompt
would be automatically denied.

The script from the prompt sequence must be exported to be used during
the grace period handler function.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
The observer client will only see the first create prompt, since the
other creations are blocked until the first prompt is actioned. Thus,
remove the others from the expected sequence.

Also, use separate snapd socket clients for each scripted client so that
all are capable of seeing all notices. Otherwise, the cleanup client
can't see the first decoy prompt, and thus fails the script.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
@olivercalder

Copy link
Copy Markdown
Member Author

Related: #297

CC @natibek

gabelluardo added a commit that referenced this pull request Apr 1, 2026
This fixes hanging tests adding timeouts for `ScriptedClient` calls and
test channels. It's based on the ideas in #290 with a couple of other
workarounds.

---
UDENG-9108
@gabelluardo
gabelluardo removed their request for review April 22, 2026 10:06
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

Successfully merging this pull request may close these issues.

1 participant