canonical / prompting-client

GNU General Public License v3.0
8 stars 4 forks source link

feat(scripted-client): supporting passing templating variables and a top level filter #107

Closed sminez closed 1 month ago

sminez commented 1 month ago

See the updated sequence JSON files for examples of how this looks in practice.

A full example "spread" test for a happy path read can be written as follows:

#!/usr/bin/env sh

PREFIX="$1"
TEST_DIR="/home/ubuntu/test/$PREFIX"

prompting-client.scripted \
  --script="$TEST_DIR/prompt-sequence.json" \
  --var "BASE_PATH:$TEST_DIR" | tee "$TEST_DIR/outfile" &

# Ensure that the test client is already listening
sleep 0.2
TEST_OUTPUT="$(aa-prompting-test.read "$PREFIX")"

# Ensure that the test client has time to write its output
# For tests with a grace period this will need to be taken into account as well
sleep 0.2
CLIENT_OUTPUT="$(cat "$TEST_DIR/outfile")"

if [ "$CLIENT_OUTPUT" != "success" ]; then
  echo "test failed"
  echo "output='$CLIENT_OUTPUT'"
  exit 1
fi

if [ "$TEST_OUTPUT" != "testing testing 1 2 3" ]; then
  echo "test script failed"
  exit 1
fi

Example output

I'm not sure what to make about this error that I see from update.go @olivercalder? I'm assuming it is coming from snapd but I'm not sure why it shows up here in the output from running the scripted client...

Output when a sequence succeeds:

running 1 test
update.go:85: cannot change mount namespace according to change mount (/run/user/1000/doc/by-app/snap.prompting-client /run/user/1000/doc none bind,rw,x-snapd.ignore-missing 0 0): cannot inspect "/run/user/1000/doc": lstat /run/user/1000/doc: permission denied
creating client
script path: /home/ubuntu/test/edffdcfd-6f11-4dbd-947b-77c839cce0fa/prompt-sequence.json
success
test scripted_client_test_allow ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 28 filtered out; finished in 0.47s

Output when a sequence fails:

running 1 test
update.go:85: cannot change mount namespace according to change mount (/run/user/1000/doc/by-app/snap.prompting-client /run/user/1000/doc none bind,rw,x-snapd.ignore-missing 0 0): cannot inspect "/run/user/1000/doc": lstat /run/user/1000/doc: permission denied
creating client
script path: /home/ubuntu/test/e61b1d06-8cbb-41b0-9cb2-2aedba30e4c1/prompt-sequence.json
failed prompt sequence: prompt 0 did not match the provided sequence: [MatchFailure { field: "requested_permissions", expected: "[\"write\"]", seen: "[\"read\"]" }]

script: {
  "version": 1,
  "prompt-filter": {
    "snap": "aa-prompting-test",
    "interface": "home",
    "constraints": {
      "path": "/home/ubuntu/test/e61b1d06-8cbb-41b0-9cb2-2aedba30e4c1/.*"
    }
  },
  "prompts": [
    {
      "prompt-filter": {
        "constraints": {
          "path": ".*/test.txt",
          "requested-permissions": [ "write" ]
        }
      },
      "reply": {
        "action": "allow",
        "lifespan": "single",
        "constraints": {
          "path-pattern": "/home/ubuntu/test/e61b1d06-8cbb-41b0-9cb2-2aedba30e4c1/test.txt",
          "permissions": [ "read" ]
        }
      }
    }
  ]
}
sminez commented 1 month ago

LGTM :tada:

A thought I've had coming back to the test setup and having to refresh my memory is it might be worth putting a HOW-TO section in the README running through how the scripted client and testing snaps are intended to be used in a future commit.

Good point. There is some existing documentation for the scripted client in the docs directory but it's incorrect now following these changes. I'll get that fixed up. The testing snap though is only intended for use within the integration tests, so I'm not sure if that should be included in the docs?

matthew-hagemann commented 1 month ago

Good point. There is some existing documentation for the scripted client in the docs directory but it's incorrect now following these changes. I'll get that fixed up. The testing snap though is only intended for use within the integration tests, so I'm not sure if that should be included in the docs?

I'm mainly thinking if someone from snapd needed to update the spread tests and didn't have much context on what was where. So the aa-prompting-test and integration-tests artifacts we upload.

sminez commented 1 month ago

I'm mainly thinking if someone from snapd needed to update the spread tests and didn't have much context on what was where. So the aa-prompting-test and integration-tests artifacts we upload.

Ah ok, well the integration-tests binary usage in snapd is going to be replaced by re-implementing those tests using the scripted client. aa-prompting-test is a little more interesting as even if they don't use that specific snap directly, they'll want to use something similar. I'm actually tempted to update it to just accept a shell command and then run that from inside of the snap so that it triggers prompts. I'll look at adding that to this PR while we wait for input from @olivercalder on the snapd side of things :slightly_smiling_face:

olivercalder commented 1 month ago

I'm actually tempted to update it to just accept a shell command and then run that from inside of the snap so that it triggers prompts.

You can do this with any snap actually, just do snap run --shell <snapname> -c 'echo hello there'. So I think all we'd need for something like this would be a stub snap with the "home" interface (don't even need apps probably), then we can do everything via snap run --shell.