Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit and integration tests for pebble run #351

Closed
hpidcock opened this issue Feb 9, 2024 · 4 comments
Closed

Add unit and integration tests for pebble run #351

hpidcock opened this issue Feb 9, 2024 · 4 comments
Labels
24.10 Planned for the 24.10 cycle

Comments

@hpidcock
Copy link
Member

hpidcock commented Feb 9, 2024

Currently pebble run has no unit tests or integration tests. Considering this is the main entry point, it would be valuable to add integration testing here.

There is a few possible ways we could achieve this:

  • Use spread to define the integration tests.
  • Use python or go to write integration tests using a test framework.
  • Write a bunch of shell scripts.

The integration tests should cover all the expected use cases of pebble at the moment.

@benhoyt
Copy link
Contributor

benhoyt commented Feb 13, 2024

Just for the record, @hpidcock and I talked briefly about this, and I'm quite keen to try using go test with a build tag for this, for example go test -tags integration ./internals/testintegration. This will mean we can run some nice high-level tests of pebble run without installing any additional tooling.

benhoyt pushed a commit that referenced this issue Feb 23, 2024
`$PEBBLE_COPY_ONCE` environment variable allows `pebble run` to copy layers and state from another directory when seeding a new `$PEBBLE` directory.

The rationale for this change is to allow OCI images to specify existing layers in the default `$PEBBLE` location. Then at runtime, be able to run `pebble` with a different `$PEBBLE` directory. This allows `pebble` to be run as a non-root user or run on a read-only root filesystem.

Issue #351 outlines that `pebble run` has no unit or integration tests. A test should be performed manually for now.

Manual QA is as follows:
- Create a directory, with a layers subdirectory and valid testing layers (e.g. ~/dir/with-existing-state)
- Create another empty directory. (e.g. ~/dir/with-no-state)
- `PEBBLE_COPY_ONCE=~/dir/with-existing-state  PEBBLE=~/dir/with-no-state pebble run`
- Check all the layers are loaded.
- Check all the layers are copied.
- Modify a layer in `~/dir/with-no-state/layers`.
- Restart `pebble run` and check that it doesn't crash.
- Check that the modifications are preserved after the restart (PEBBLE_COPY_ONCE is only for an empty directory).
- Check `PEBBLE_COPY_ONCE` can be set to a non-existing directory with `pebble run` passed `--create-dirs`

[JU090](https://docs.google.com/document/d/1NWV4QsYq1NldS_V_YlafpJQyNAV-WS46VsIKGBrin_s/edit)
[JUJU-5435](https://warthogs.atlassian.net/browse/JUJU-5435)

[JUJU-5435]: https://warthogs.atlassian.net/browse/JUJU-5435?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
@benhoyt benhoyt changed the title pebble run has no unit tests or integration tests Add unit and integration tests for pebble run Mar 13, 2024
@benhoyt benhoyt added the 24.10 Planned for the 24.10 cycle label Mar 13, 2024
@benhoyt
Copy link
Contributor

benhoyt commented Apr 18, 2024

Relatedly, we should figure out a better way to mark and run "root tests" -- or maybe these just become integration tests instead.

name: Root Tests
steps:
- uses: actions/checkout@v3
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: ${{ matrix.go }}
- name: Test
run: |
go test -c ./internals/daemon
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^execSuite\.TestUserGroup$
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^execSuite\.TestUserIDGroupID$
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^filesSuite\.TestWriteUserGroupReal$
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^filesSuite\.TestMakeDirsUserGroupReal$
go test -c ./internals/overlord/servstate/
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./servstate.test -check.v -check.f ^S.TestUserGroup$

@IronCore864
Copy link
Contributor

I have finished researching the possible solutions mentioned above, and here are some of my thoughts:

  1. Shell scripts: I did not test it but I'm pretty sure this would definitely work. I gave up this option because I don't think shell scripts can be as easily maintained as other options, like Python scripts with some sort of testing framework.
  2. Python/Go to write integration tests: This is where I put my major effort. I chose Go because I don't want to introduce another tech into this repo (Well, technically, Python is already introduced in the documentation of Pebble but it's not used for development-related tasks yet, so I don't want to increase the complexity of the tech stack). I followed Ben's suggestion with Go Build Flags. I have done a PoC, see here.
  3. Use spread for integration tests: I've tried it out with LXD backend, and I think this is similar to Dima's idea when we chatted about this in our 1-1 where he suggested that maybe we could run Pebble as root in a container for integration tests and other root tests. I think it's a good idea but it brings extra dependencies, like spread+LXD or docker and other test scripts.

So, after investigation, I think No.2 probably is the best choice here.

A few more words on the build flags (also known as the build constraints) PoC:

  1. It seems the test suite can't be used with build flags, it can't collect any tests: testing: warning: no tests to run. Maybe this is possible but I haven't made it work yet, and I think this could be the reason: as per the Golang Doc, the build tag lists the conditions under which a file should be included in the package, and maybe this is the reason why it doesn't work with suite. It means we can't do something like func (s *IntegrationSuite) TestXXX(c *C), but can only do func TestXXX(t *testing.T); which is OK I think, just something worth pointing out.
  2. In the PoC I built the binary in setup, then created some functions to help create layers, run pebble and return logs. The logs part is tricky because the daemon is a long-running process and we can't wait for it to "finish". Here I used something like "if there are no new logs in the past second, kill the process and return". Maybe this isn't ideal. I had another draft where I simply passed a value for timeout into it, sleep, then kill. Both worked, but I'm not sure if this is the best we can do here.

@benhoyt
Copy link
Contributor

benhoyt commented Sep 24, 2024

This is done now in #497.

@benhoyt benhoyt closed this as completed Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
24.10 Planned for the 24.10 cycle
Projects
None yet
Development

No branches or pull requests

3 participants