[PATCH] D63935: [llvm-ar][test] Increase llvm-ar test coverage

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: test/tools/llvm-ar/create.test:1
+# Test the creation warning and supression of that warning.
+
----------------
gbreynoo wrote:
> MaskRay wrote:
> > Since you've used `## ` below..
> > 
> > `#` -> `##`
> The times I've used `##` below has been to make comments stand out more in files that require the use of `#` on each line. Would it be preferred to standardise the use of `##` and `#` on all files regardless of their use as input YAML files?
>From what I can tell in llvm-objcopy, llvm-objdump, ..., comments in the header are also prefixed with `## ` nowadays.


================
Comment at: test/tools/llvm-ar/create.test:8
+RUN: llvm-ar r %t.warning.ar %t1.txt %t2.txt 2>&1 \
+RUN:   | FileCheck %s --check-prefix=WARNING
+
----------------
No strong preference here, but if you like it:

```
RUN: rm -f %t.warning.ar
RUN: llvm-ar r %t.warning.ar %t1.txt %t2.txt 2>&1 \
RUN:   | FileCheck %s --check-prefix=WARNING -DOUTPUT=%t.warning.ar

WARNING: warning: creating [[OUTPUT]]
```


================
Comment at: test/tools/llvm-ar/create.test:12
+RUN: llvm-ar cr %t.supressed.ar %t1.txt %t2.txt 2>&1 \
+RUN:   | FileCheck --allow-empty %s --check-prefix=SUPRESSED
+
----------------
`FileCheck --allow-empty /dev/null --implicit-check-not={{.}}`


================
Comment at: test/tools/llvm-ar/dash-before-letter.test:1
+# Test the use of dash before key letters.
+
----------------
Can you merge this test with default-add.test ?


================
Comment at: test/tools/llvm-ar/extract.test:7
+RUN: llvm-ar x %t/empty.a 2>&1 \
+RUN:   | FileCheck --allow-empty %s --implicit-check-not {{.}}
+
----------------
gbreynoo wrote:
> MaskRay wrote:
> > `--implicit-check-not {{.}}` does not check a regular expression.
> > 
> > Use `| count 0` instead
> I did some experimentation with implicit-check-not and a regular expression and it seemed to work fine, do you have a test case that illustrates the issue?
Thanks for clarification. It works.

`FileCheck --allow-empty /dev/null --implicit-check-not={{.}}`


================
Comment at: test/tools/llvm-ar/extract.test:13
+
+# Single member:
+RUN: cd %t/extracted && llvm-ar x %t/archive.a a.txt
----------------
##


================
Comment at: test/tools/llvm-ar/move-after.test:31
+RUN: llvm-ar rc %t-multiple.a %t1.txt %t2.txt %t3.txt %t4.txt
+RUN: llvm-ar ma %t1.txt %t-multiple.a %t3.txt %t4.txt
+RUN: llvm-ar t %t-multiple.a | FileCheck %s --check-prefix=MULTIPLE
----------------
It may be interesting to specifies files not in the order:

`llvm-ar ma %t1.txt %t-multiple.a %t4.txt %t3.txt`


================
Comment at: test/tools/llvm-ar/preserve-mod-time.test:7
+
+RUN: env TZ=GMT touch -t 200001020304 %t/a.txt
+RUN: llvm-ar rcU %t/timestamp.a %t/a.txt
----------------
`TZ=UTC`




================
Comment at: test/tools/llvm-ar/preserve-mod-time.test:10
+RUN: cd %t/extracted && llvm-ar xo %t/timestamp.a
+RUN: env TZ=GMT ls %t/extracted --full-time | FileCheck %s
+
----------------
`--full-time` is specific to GNU coreutils.

See llvm-objcopy/ELF/strip-preserve-mtime.test, check just the yyyy part should be ok:

```
# CHECK-PRESERVE-MTIME:        {{[[:space:]]1997}}
```



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63935/new/

https://reviews.llvm.org/D63935