Skip to content

Commit

Permalink
antlir/testing/image_test: Allow hyphen flags in custom_unittest args
Browse files Browse the repository at this point in the history
Summary:
- Currently, if a user specifies any `--option` flags in their `custom_unittest` argument list, they get parsed as options in `Test`, rather than `command: Vec<String>`
- I see that some of the other types types already have this option, so I *assume* it's probably safe to add here?
- Most people are using `image_sh_test` afaik with antlir, which just passes a single shell script. But I added an `mtiavm.custom_unittest` wrapper that will rely on this.

Test Plan:
# image_test unittests

## Before

Add test, see it fail:

```
$ buck test @//mode/opt :
File changed: fbcode//antlir/antlir2/testing/image_test/src/lib.rs
File changed: fbcode//antlir/antlir2/testing/image_test/src/lib.rsvocvgF.bck
⚠ Fatal: antlir/antlir2/testing/image_test:image_test_lib-unittest - test::test_custom (0.0s)
Test appears to have passed but the binary exited with a non-zero exit code.
This usually means something has crashed after the test was done.
stdout:
{ "type": "suite", "event": "started", "test_count": 1 }
{ "type": "test", "event": "started", "name": "test::test_custom" }

stderr:
error: unexpected argument '--list' found

  tip: to pass '--list' as a value, use '-- --list'

Usage: test custom <TEST_CMD>

For more information, try '--help'.

exit_code:"Process terminated by the signal: 2"
Buck UI: https://www.internalfb.com/buck2/828bf8f3-f106-4d20-8fdd-ec41339657d5
Test UI: https://www.internalfb.com/intern/testinfra/testrun/9288674282929750
Network: Up: 5.4KiB  Down: 1.4KiB  (reSessionID-b2a8388a-4ea4-4c3d-ba68-65cbc3ff4d92)
Jobs completed: 25. Time elapsed: 6.7s.
Cache hits: 0%. Commands: 4 (cached: 0, remote: 0, local: 4)
Tests finished: Pass 5. Fail 0. Fatal 1. Skip 0. Build failure 0
1 TESTS FATALS
  ⚠ antlir/antlir2/testing/image_test:image_test_lib-unittest - test::test_custom
```

## After
```
$ buck test @//mode/opt //antlir/antlir2/testing/image_test:
Buck UI: https://www.internalfb.com/buck2/cf7ef55b-0afd-4b06-b7fd-aaf453a0634a
Test UI: https://www.internalfb.com/intern/testinfra/testrun/562950337434618
Network: Up: 19KiB  Down: 15MiB  (reSessionID-2d74ebb0-856e-41ca-9a5e-27edc7249255)
Jobs completed: 3111. Time elapsed: 6.9s.
Cache hits: 99%. Commands: 579 (cached: 574, remote: 0, local: 5)
Tests finished: Pass 6. Fail 0. Fatal 0. Skip 0. Build failure 0
```

# tupperware/sudotest

```
Buck UI: https://www.internalfb.com/buck2/9bb95e97-fb7c-4301-ad62-31e10f90304b
Test UI: https://www.internalfb.com/intern/testinfra/testrun/15481123770302738
Network: Up: 117KiB  Down: 1.5MiB  (reSessionID-b3c19bd8-7785-42e7-ac46-c4335ea4dae8)
Jobs completed: 596. Time elapsed: 4.5s.
Cache hits: 98%. Commands: 127 (cached: 124, remote: 0, local: 3)
Tests finished: Pass 6. Fail 0. Fatal 0. Skip 1. Build failure 0
```

## mtiavm.custom_unittest fix

```
$ buck test @//mode/opt :test-null-kernel-iris-qmodel-usd
Buck UI: https://www.internalfb.com/buck2/0ac9bc8e-1790-44ef-909c-cf749dfbd5f6
Test UI: https://www.internalfb.com/intern/testinfra/testrun/4503599876140958
Network: Up: 185MiB  Down: 827MiB  (reSessionID-9e21ef27-4b7c-4da0-94f2-12db98b0838a)
Jobs completed: 428158. Time elapsed: 2:00.0s.
Cache hits: 99%. Commands: 13485 (cached: 13401, remote: 0, local: 84). Fallback: 6/84
Tests finished: Pass 1. Fail 0. Fatal 0. Skip 0. Build failure 0
```

```
$ buck run @//mode/opt :test-null-kernel-iris-qmodel-usd --emit-shell | tr ' ' '\n'
/home/pdel/fbsource/buck-out/v2/gen/fbcode/f0b024372987e0e1/mtia/vm/__mtiavm__/mtiavm
test
--config
/home/pdel/fbsource/buck-out/v2/gen/fbcode/76bc92745ac4183e/mtia/vm/__iris-qmodel-usd__/vm.json
'--env=FBCODE_BUILD_TOOL="buck"'
'--env=FBCODE_BUILD_MODE="opt"'
custom
/home/pdel/fbsource/buck-out/v2/gen/fbcode/76bc92745ac4183e/apsw/tools/mtiaclient/__mtiaclient__/mtiaclient
pe-job
'--output-dir=/tmp/out'
'--program=/home/pdel/fbsource/buck-out/v2/gen/fbcode/968817d3c6a8ee23/apsw/test_pe_programs/__noop__/noop'
'--num-iters=100'
```

# rpm integrity test command

Making sure the rpm integrity tests pass (since I'm removing the `--` hyphen from the rule command)

```
$ buck test @//mode/opt //antlir/antlir2/testing/tests/...
Buck UI: https://www.internalfb.com/buck2/d2cad15d-98c1-47c0-af20-d9cec94e7e54
Test UI: https://www.internalfb.com/intern/testinfra/testrun/2814750019329094
Network: Up: 1.0MiB  Down: 204MiB  (reSessionID-cc3f4998-cf19-44b0-8a6f-290e63a5034b)
Jobs completed: 98008. Time elapsed: 25.7s.
Cache hits: 82%. Commands: 2525 (cached: 2082, remote: 0, local: 443). Fallback: 2/443
Tests finished: Pass 101. Fail 0. Fatal 0. Skip 0. Build failure 0
```

## Before

```
$ buck run @//mode/opt :test-foo-rpm-integrity --emit-shell | tr ' ' '\n'
/home/pdel/fbsource/buck-out/v2/gen/fbcode/f0b024372987e0e1/antlir/antlir2/testing/image_test/__image-test__/image_test
'--spec=/home/pdel/fbsource/buck-out/v2/gen/fbcode/01b1791af62c894e/antlir/antlir2/testing/tests/__test-foo-rpm-integrity__/spec.json'
custom
/home/pdel/fbsource/buck-out/v2/gen/fbcode/f0b024372987e0e1/antlir/antlir2/testing/image_rpms_test/__image-rpms-test__/image_rpms_test
integrity
--
'--layer=/layer'
```

## After

The rpm integrity test command now omits the `--` hyphen, and flag parsing depends on relative position within the command:

```
$ buck run @//mode/opt :test-foo-rpm-integrity --emit-shell | tr ' ' '\n'
/home/pdel/fbsource/buck-out/v2/gen/fbcode/f0b024372987e0e1/antlir/antlir2/testing/image_test/__image-test__/image_test
'--spec=/home/pdel/fbsource/buck-out/v2/gen/fbcode/01b1791af62c894e/antlir/antlir2/testing/tests/__test-foo-rpm-integrity__/spec.json'
custom
/home/pdel/fbsource/buck-out/v2/gen/fbcode/f0b024372987e0e1/antlir/antlir2/testing/image_rpms_test/__image-rpms-test__/image_rpms_test
integrity
'--layer=/layer'
```

# metaroce vm test

## After

```
$ buck test //neteng/ai/orion/metarxe/vmtest:sh
Buck UI: https://www.internalfb.com/buck2/66f8cfe3-c8f3-41c2-b3a9-299322976016
Test UI: https://www.internalfb.com/intern/testinfra/testrun/14355223863675739
Network: Up: 508MiB  Down: 8.7GiB  (reSessionID-7d6745ff-e4b1-4119-b51b-6a67111e9948)
Jobs completed: 807841. Time elapsed: 2:46.9s.
Cache hits: 99%. Commands: 46992 (cached: 46709, remote: 0, local: 283). Fallback: 6/283
Tests finished: Pass 1. Fail 0. Fatal 0. Skip 0. Build failure 0
```

Reviewed By: cyberang3l

Differential Revision: D63487659

fbshipit-source-id: c28a8c449e5649f1504c3abbc74f4b6361039013
  • Loading branch information
peterdelevoryas authored and facebook-github-bot committed Oct 1, 2024
1 parent c43ed0c commit f3fbb15
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 7 deletions.
5 changes: 0 additions & 5 deletions antlir/antlir2/testing/image_rpms_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,6 @@ def _rpm_integrity_test_impl(ctx: AnalysisContext) -> list[Provider]:
RunInfo(cmd_args(
ctx.attrs.image_rpms_test[RunInfo],
"integrity",
# image_test generally does not add this because it needs to parse
# options intended for the end test in the case of things like
# python_unittest. Add -- explicitly so that `image-test` does not
# try to parse our rpm test options
"--",
cmd_args(ctx.attrs.ignored_files, format = "--ignored-file={}"),
cmd_args(ctx.attrs.ignored_rpms, format = "--ignored-rpm={}"),
"--layer=/layer",
Expand Down
5 changes: 3 additions & 2 deletions antlir/antlir2/testing/image_test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use thiserror::Error;
/// from the command. This enum parses the expected flags for each type.
pub enum Test {
Custom {
#[clap(allow_hyphen_values = true)]
test_cmd: Vec<OsString>,
},
Gtest {
Expand Down Expand Up @@ -285,10 +286,10 @@ mod test {

#[test]
fn test_custom() {
let arg = TestArgs::parse_from(["test", "custom", "whatever"]);
let arg = TestArgs::parse_from(["test", "custom", "whatever", "--list"]);
assert!(!arg.test.is_list_tests());
assert_eq!(arg.test.output_dirs(), HashSet::new());
assert_eq!(arg.test.into_inner_cmd(), vec!["whatever"]);
assert_eq!(arg.test.into_inner_cmd(), vec!["whatever", "--list"]);
}

#[test]
Expand Down

0 comments on commit f3fbb15

Please sign in to comment.