Skip to content

Initial LISAv3 via Pytest review#1064

Closed
andyleejordan wants to merge 61 commits into
masterfrom
andschwa/initial-pytest-review
Closed

Initial LISAv3 via Pytest review#1064
andyleejordan wants to merge 61 commits into
masterfrom
andschwa/initial-pytest-review

Conversation

@andyleejordan
Copy link
Copy Markdown
Member

I created the branch pytest/main and enabled branch protections on it so all code goes through review. This PR supersedes #1044 which was an initial demo. There are TODOs throughout, which I'd like to work with @lpereira to prioritize and work-on to finish this proof-of-concept, but I'd like to get what does exist reviewed before it goes any further. I have attempted to document design decisions and alternatives considered in pytest/README.md, and have setup some basic CI with a self-test.

This is a _working_ test.
Extend Fabric’s Connection class with a new (and simple) command “cat”
which return the value of a remote file as a string.

Setup a Config for the Connection when creating it that echoes every
command, disables the stdin forwarding (since we’re running under
Pytest), and fixes the PATH since the remote commands don’t run under a
login shell.
This could be extended to instead deploy a host of the specified distro.
Most likely we’ll want a command-line parameter that the fixture uses to
create a Node with the given requirements, and then tests will be
skipped if their requirements aren’t met.

Further more, the mark here is very simple. It can instead take keyword
arguments, which would map to our metadata.
As supplying types for these would be supremely annoying.
Accidentally eliminated Fabric’s default overrides of Invoke by
supplying my own config based on `invoke.Config` to Fabric. Oops.
Tenacity’s `stop_after_delay` seems to get confused underneath pytest,
applying the delay as a total to all code using it, meaning the last
test `get_boot_diagnostics` started always failing due to being
canceled. There appears to be a similar issue (though fixed) in the
library for async functions, where the state on their `RetryState`
object doesn’t get reset properly.
This reverts commit a9e4aff.

The plugin is incompatible with class fixtures, so when the last test
in a test class fails, the fixture is torn down (in this case, deleting
the node) before the test is retried, which doesn’t work for us.
Note that `warn=True` causes timeouts to be ignored. On some images, the
invoked program (it’s not even a shell) just hangs waiting for manual
input. So we set a proper timeout.
They can serve completely different purposes. In this case, re-running
the whole test and reporting it as a second test run.
@andyleejordan
Copy link
Copy Markdown
Member Author

Dang it, wrong target branch.

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