On 25/03/2023 13:16, René Scharfe wrote:
fd2da4b1ea (archive: add --mtime, 2023-02-18) added a helper function
for checking the file modification time of an extracted entry. Use it
for the older mtime test as well to shorten the code and piggyback on
the archive extraction done to validate file contents.
Signed-off-by: René Scharfe <l.s.r@xxxxxx>
---
t/t5000-tar-tree.sh | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 918a2fc7c6..f0bd70dbd6 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -185,6 +185,7 @@ test_expect_success 'git archive' '
'
check_tar b
+check_mtime b a/a 1117231200
test_expect_success 'git archive --mtime' '
git archive --mtime=2002-02-02T02:02:02-0200 HEAD >with_mtime.tar
@@ -257,14 +258,6 @@ test_expect_success 'git archive --remote with configured remote' '
test_cmp_bin b.tar b5-nick.tar
'
-test_expect_success 'validate file modification time' '
- mkdir extract &&
- "$TAR" xf b.tar -C extract a/a &&
- test-tool chmtime --get extract/a/a >b.mtime &&
- echo "1117231200" >expected.mtime &&
- test_cmp expected.mtime b.mtime
-'
-
test_expect_success 'git get-tar-commit-id' '
git get-tar-commit-id <b.tar >actual &&
git rev-parse HEAD >expect &&
--
2.40.0
This patch looks good to me.
When reading check_mtime() I got confused by extra space in first parameter
to test_expect_success, but after running t5000 and reading check_tar, it'd
become obvious that the space is there to align together subtests related
to one invocation of git-archive:
[...]
ok 7 - remove ignored file
ok 8 - git archive
ok 9 - extract tar archive
ok 10 # skip interpret pax headers (missing TAR_NEEDS_PAX_FALLBACK)
ok 11 - validate filenames
ok 12 - validate file contents
ok 13 - validate mtime of a/a
ok 14 - git archive --mtime
ok 15 - extract tar archive
ok 16 # skip interpret pax headers (missing TAR_NEEDS_PAX_FALLBACK)
ok 17 - validate filenames
ok 18 - validate file contents
ok 19 - validate mtime of a/a
ok 20 - git archive --prefix=prefix/
[...]
The only tangentially related nitpick is to the function check_mtime(),
which doesn't follow the code style for Shell scripts -- a space is missing
before parentheses. Same for almost all the other helper functions in test
files related to git-archive:
$ git grep -E '^[a-z_]+[(][)]' t/t500*
t/t5000-tar-tree.sh:get_pax_header() {
t/t5000-tar-tree.sh:check_tar() {
t/t5000-tar-tree.sh:check_added() {
t/t5000-tar-tree.sh:check_mtime() {
t/t5001-archive-attr.sh:test_expect_exists() {
t/t5001-archive-attr.sh:test_expect_missing() {
t/t5002-archive-attr-pattern.sh:test_expect_exists() {
t/t5002-archive-attr-pattern.sh:test_expect_missing() {
t/t5003-archive-zip.sh:check_zip() {
t/t5003-archive-zip.sh:check_added() {
t/t5004-archive-corner-cases.sh:make_dir() {
t/t5004-archive-corner-cases.sh:check_dir() {
t/t5004-archive-corner-cases.sh:build_tree() {
compare to:
$ git grep -E '^[a-z_]+ [(][)]' t/t500*
t/t5000-tar-tree.sh:tar_info () {
t/t5001-archive-attr.sh:extract_tar_to_dir () {
Quote from Documentation/CodingGuidelines:
- We prefer a space between the function name and the parentheses,
and no space inside the parentheses. The opening "{" should also
be on the same line.
(incorrect)
my_function(){
...
(correct)
my_function () {
...
