Repository navigation
Conversation
brainstorm
left a comment
brainstorm
left a comment
There was a problem hiding this comment.
Thanks @jubeormk1!
I have no objections on the general idea of having a "scenarios run recipe" and creating this end to end harness as a whole. That's what we desperately need in HIL testing right now.
As you see in my comments, it's only the parsing bits (and in a lesser degree the YAML format) where I see areas for improvement before merging. OTOH, I see the value of getting this to work now-now since we need better HIL.
What do you think? Should we refactor later or try to get the boot output in machine-readable JSON so that both humans and machines can read those settings and so your code parses better?
There was a problem hiding this comment.
Can we switch to .toml instead of yaml? SSH Stamp already uses .toml for board definitions (and .json in some other contexts), but I think that we can define this "actions" yaml easily in .toml instead?
| .context("espflash reset failed") | ||
| } | ||
|
|
||
| fn parse_ssh_stamp_network_from_serial(serial: &Serial, context: &mut ContextState) -> Result<()> { |
There was a problem hiding this comment.
I reckon that all this adhoc parsing can be better addressed if and when #123 is reviewed (and eventually merge?), please take a peek at it, but in a nutshell implements JSON output for the boot messages so that it's relatively easy for it to be machine-readable.
| let parsed = parse_network_lines(&lines).unwrap(); | ||
| assert_eq!(parsed.ssid, "test-ssid"); | ||
| assert_eq!(parsed.psk, "test-password"); | ||
| assert_eq!(parsed.ip, "192.168.4.1".parse::<Ipv4Addr>().unwrap()); |
There was a problem hiding this comment.
We need to talk about our hardcoded 192.168.4.1 :_/
The intention is to explore how to define multi-step flows that might require keeping a serial connection alive, parsing information and stablishing ssh connections. This PR is aimed at doing an integration test for the first set of actions.
Removing redundant action and refactoring the action match into smaller handlers for clarity.
Scenarios are now plain Rust functions selected with `--scenario` (defaulting to `wifi-ap`), instead of a YAML list of actions, which drops the serde_yaml dependency. The SSID, PSK and AP address come from the `boot` and `net_up` JSON objects the firmware prints since brainstorm#123, rather than scraping log lines, so the AP address is no longer assumed. Objects from before the reset are ignored, and an unknown JSON schema version fails the run. Also align with the other commands: `--port` instead of `--prg-serial`, and no separate join step since `wait_for_reachable` already joins. The unrelated host.rs change is dropped.
7dd18bf to
dd33c10
Compare
Starting e2e
This is the first contribution, open for discussion, to migrate and expand e2e testing to xtask. I believe that it requires more work, but I would like to discuss it an what is better than a Pull Request
coffeeto discuss code.E2E tests scripting
We need a way to store and organise steps for E2E tests that are easy to visualise and can do abstract things. For that I decided that having a dedicated xtask e2e that admits scenarios can be a good fit.
This sketch is barebones reduced to prove how this could work:
$ cargo xtask e2e board=<target board> prg-serial=</dev/ttyANN> interface=<nmcli interface id> scenario=<path to scenario> verboseAll arguments long, because this is better readable on a CI script.
A scenario could be as simple as wifi_ap_mode.yml which includes an order set of actions, defined by the enum
Actionand evolve a context state.Missing bits and pieces
Argsvariants to cover things such as a serial adapter path to emulate the target device the ssh-stamp will be receiving/sending information from.