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

proposal: use vmtest as go test runner #3

Open
lmb opened this issue May 10, 2023 · 5 comments
Open

proposal: use vmtest as go test runner #3

lmb opened this issue May 10, 2023 · 5 comments

Comments

@lmb
Copy link

lmb commented May 10, 2023

I'd like to be able to do the following:

$ go test -exec vmtest ./... -vm.image /path/to/vmlinux -vm.env SOME_ENV -other-flag

Behind the scenes:

  1. Compile the tests into a binary in /tmp (?)
  2. Execute vmtest with the following args: "/tmp/path/to/test" "-vm.image" "/path/to/vmlinux" "-vm.env" "SOME_ENV" "-other-flag"
  3. vmtest would make the appropriate 9pfs mounts to make the go test runner happy: https://github.com/cilium/ebpf/blob/master/run-tests.sh#L27-L78
  4. vmtest would execute the test with /tmp/path/to/test -other-flag, with PWD being the directory vmtest is invoked in on the host

P.S. The weird vm. prefix on flags is needed due to how go tests behave when unknown flags are present. Behind the scenes vmtest should strip the prefix.

It would also be nice to be able to specify certain "global settings" in vmtest.toml, like memory or number of CPUs.

My goal would be to replace most of run-tests.sh with vmtest.

@danobi
Copy link
Owner

danobi commented May 11, 2023

Assuming we add the one-liner interface to vmtest (which I am much in favor of), wouldn’t it be possible to pass flags to vmtest like this? Passing unknown flags to go test seems a bit odd.

$ go test -exec “vmtest —kernel /path/to/vmlinux”

My test shows:

$ go test -exec "./wrapper/a.out --wrapper-flag"                                                                                     
argv[0] = ./wrapper/a.out                                                                                                                            
argv[1] = --wrapper-flag                                                                                                                             
argv[2] = /tmp/go-build2128986016/b001/test.test                                                                                                     
argv[3] = -test.paniconexit0                                                                                                                         
argv[4] = -test.timeout=10m0s                                                                                                                        
ok      example.com/test        0.001s

wrapper/main.c: https://pastes.dxuuu.xyz/l9knkt

It seems the same technique could be used for rust: https://github.com/libbpf/libbpf-rs/blob/4574a7825f1c088726c3d3eefd3d0d8f9ddd116e/libbpf-rs/.cargo/config#L2

@danobi
Copy link
Owner

danobi commented May 11, 2023

It would also be nice to be able to specify certain "global settings" in vmtest.toml, like memory or number of CPUs.

That seems reasonable to me. The only thing I’m not sure about is what happens if you ask QEMU for more resources than is available on the current machine. For example, if you ask for 4 cpus on a 2 core machine. Assuming QEMU errors out, should we clamp down to 2 or return an error? It’s more of a instance property than a config file property, so maybe it should go on cmd line?

Edit: looks like asking for 32 cpus on my 8 core machine works but 1T of memory on a 32G machine does not: https://pastes.dxuuu.xyz/26tn19

@lmb
Copy link
Author

lmb commented May 15, 2023

wouldn’t it be possible to pass flags to vmtest like this?

Yep, this works, but now I have to deal with double-quoting on the cmdline, which isn't very nice. Stripping out extra args gets rid of that problem.

@lmb
Copy link
Author

lmb commented May 15, 2023

It’s more of a instance property than a config file property, so maybe it should go on cmd line?

Good point, not sure there is a good answer. For my use case I have some tests which require > 1 cpu IIRC.

@danobi
Copy link
Owner

danobi commented May 15, 2023

Ok, I'll give supporting the go-style flags a shot -- UX is important. Hopefully it's possible to use clap's custom parser support to extend. Would be unfortunate to throw clap out.

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 a pull request may close this issue.

2 participants