Resolve local paths against the suite file and widen --check - #97
Merged
Conversation
Five behaviours the documentation had to apologize for, fixed instead. Local paths now follow one convention. Absolute paths are used as-is, `~` expands to the invoking user's home, and everything else resolves against the directory holding the suite file rather than the process working directory. That covers file-step sources and destinations, docker volumes, LXD disk sources, SSH keys and known_hosts, LXD certificates, and compose_file, joining dockerfile and !!load_from which already worked this way. A suite now behaves the same run from the repository root, from its own directory, or from a CI checkout elsewhere. --check gained the validation it was assumed to have: - Required fields: host on ssh, image on docker, compose_file on docker-compose. Previously an ssh node missing host passed the check and failed the run dialling :22. - Unknown option names, reported with the accepted set. Options decode through a JSON round-trip that discards unrecognised keys, so `privilaged` for `privileged` left the option at its default while the suite read as though it were set. - Cross-node constraints — duplicate names and more than one local node — which lived in the node factory and so never ran under --check. Two options that were missing: - docker `command` and `entrypoint` override the image's CMD and ENTRYPOINT, so a bare distribution image can host a node instead of exiting at once. - docker `container_name` and lxd `instance_name` decouple the platform identifier from node identity, defaulting to the node name. The node name stays what `node:` references and what reports show.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five things the documentation had to apologize for. Rather than document the wart, fixed the behaviour.
One convention for local paths
Absolute paths are used as-is,
~expands to the invoking user's home, and everything else resolves against the directory holding the suite file — not the process working directory.That now covers file-step
source/dest, dockervolumes, LXD disksource, SSHkeyandknown_hosts, LXD client/server certificates, andcompose_file, joiningdocker.images[].dockerfileand!!load_fromwhich already worked this way.A suite is portable as a result. Verified by running the same suite from
/:Before this, that same run failed with
open fixtures/app.conf: no such file or directory.--checknow validates what it was assumed tohostonssh,imageondocker,compose_fileondocker-compose. Ansshnode missinghostused to pass the check and then fail the run dialling:22.privilagedleftprivilegedat its default while the suite read as though it were set — a silent false green.localnode. Both lived inline in the node factory, so--check(which substitutes mock nodes) never reached them. Extracted toValidateNodeSet, called from both.Two missing options
command/entrypointon docker nodes override the image'sCMD/ENTRYPOINT. A bare distribution image can now host a node —command: ["sleep", "infinity"]— instead of exiting the moment it starts.container_name(docker) andinstance_name(lxd) decouple the platform identifier from node identity. Both default to the node name, so nothing changes unless set. The node name remains whatnode:references, what reports show, and what the container's hostname is set to.Note on strictness
Unknown option names are now a hard error, not a warning. That is the stricter reading — it is consistent with how the project has treated other silent-no-op classes, and a stray key in an existing suite will now fail rather than be ignored. Easy to soften to a warning if you would rather not break suites carrying dead keys.
Verification
New tests cover path resolution (including
~,../, absolute, and the in-memory fallback), suite-dir stamping through load, name defaulting and override for both platforms,command/entrypointdecoding, unknown-option rejection across node types, andValidateNodeSet.go build,go vet,gofmt, and the full suite are green, and the docs site builds.Docs updated throughout — the notes describing the old behaviour are gone rather than reworded.