mbox series

[v2,0/3] Improvements for Perforce tests

Message ID cover.1722421911.git.ps@pks.im (mailing list archive)
Headers show
Series Improvements for Perforce tests | expand

Message

Patrick Steinhardt July 31, 2024, 10:37 a.m. UTC
Hi,

this is the second version of my patch series that improves Perforce
tests a bit. As with the first version, the main intent is to mark all
tests as leak free. The other two patches polish the Perforce tests such
that they work on my machine, which has a more recent version of
Perforce than what we have been testing with in CI.

As discussed, despite these patches I consider Perforce to be basically
unmaintained, and my patches do not change that. But given that I don't
think we can rip out git-p4(1) just so without any kind of announcement
up front, I still think that these patches are worth it to help my own
cause, namely plugging all memory leaks hit by tests in Git.

Changes compared to v1:

  - Fix some typos.

  - Add in-code comments explaining what the compatibility case
    statements are about.

Thanks!

Patrick

Patrick Steinhardt (3):
  t98xx: fix Perforce tests with p4d r23 and newer
  ci: update Perforce version to r23.2
  t98xx: mark Perforce tests as memory-leak free

 ci/install-dependencies.sh                  |  2 +-
 t/t9800-git-p4-basic.sh                     | 17 +++++++++++++--
 t/t9801-git-p4-branch.sh                    |  1 +
 t/t9802-git-p4-filetype.sh                  | 19 ++++++++++++++---
 t/t9803-git-p4-shell-metachars.sh           |  1 +
 t/t9804-git-p4-label.sh                     |  1 +
 t/t9805-git-p4-skip-submit-edit.sh          |  1 +
 t/t9806-git-p4-options.sh                   |  1 +
 t/t9808-git-p4-chdir.sh                     |  1 +
 t/t9809-git-p4-client-view.sh               |  1 +
 t/t9810-git-p4-rcs.sh                       |  1 +
 t/t9811-git-p4-label-import.sh              |  1 +
 t/t9812-git-p4-wildcards.sh                 |  1 +
 t/t9813-git-p4-preserve-users.sh            |  1 +
 t/t9814-git-p4-rename.sh                    |  1 +
 t/t9815-git-p4-submit-fail.sh               |  1 +
 t/t9816-git-p4-locked.sh                    |  1 +
 t/t9817-git-p4-exclude.sh                   |  1 +
 t/t9818-git-p4-block.sh                     |  1 +
 t/t9819-git-p4-case-folding.sh              |  1 +
 t/t9820-git-p4-editor-handling.sh           |  1 +
 t/t9821-git-p4-path-variations.sh           |  1 +
 t/t9822-git-p4-path-encoding.sh             |  1 +
 t/t9823-git-p4-mock-lfs.sh                  |  1 +
 t/t9825-git-p4-handle-utf16-without-bom.sh  | 23 ++++++++++++++++++---
 t/t9826-git-p4-keep-empty-commits.sh        |  1 +
 t/t9827-git-p4-change-filetype.sh           |  1 +
 t/t9828-git-p4-map-user.sh                  |  1 +
 t/t9829-git-p4-jobs.sh                      |  1 +
 t/t9830-git-p4-symlink-dir.sh               |  1 +
 t/t9831-git-p4-triggers.sh                  |  1 +
 t/t9832-unshelve.sh                         |  1 +
 t/t9833-errors.sh                           |  1 +
 t/t9834-git-p4-file-dir-bug.sh              |  1 +
 t/t9835-git-p4-metadata-encoding-python2.sh |  1 +
 t/t9836-git-p4-metadata-encoding-python3.sh |  1 +
 36 files changed, 84 insertions(+), 9 deletions(-)

Range-diff against v1:
1:  f40f62f257 ! 1:  7f06de7e6b t98xx: fix Perforce tests with p4d r23 and newer
    @@ Commit message
         Naturally, modifying the depot on disk directly is quite intimate with
         the tool and thus prone to breakage when Perforce updates the way that
         data is stored. And indeed, those tests are broken nowadays with r23 of
    -    Perforce. While a file revision was previously stored as plain file
    +    Perforce. While a file revision was previously stored as a plain file
         "depot/file,v", it is now stored in a directory "depot/file,d" with
         compression.
     
    @@ t/t9800-git-p4-basic.sh: test_expect_success 'exit when p4 fails to produce mars
      test_expect_success 'exit gracefully for p4 server errors' '
     -	test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
     -	mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden &&
    ++	# Note that newer Perforce versions started to store files
    ++	# compressed in directories. The case statement handles both
    ++	# old and new layout.
     +	case "$(echo "$db"/depot/file1*)" in
     +	*,v)
     +		test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
    @@ t/t9800-git-p4-basic.sh: test_expect_success 'exit when p4 fails to produce mars
     
      ## t/t9802-git-p4-filetype.sh ##
     @@ t/t9802-git-p4-filetype.sh: test_expect_success SYMLINKS 'empty symlink target' '
    + 		#     text
      		#     @@
      		#
    ++		# Note that newer Perforce versions started to store files
    ++		# compressed in directories. The case statement handles both
    ++		# old and new layout.
      		cd "$db/depot" &&
     -		sed "/@target1/{; s/target1/@/; n; d; }" \
     -		    empty-symlink,v >empty-symlink,v.tmp &&
    @@ t/t9825-git-p4-handle-utf16-without-bom.sh: test_expect_success 'init depot with
     -		sed -e "\$d" depot/file1,v >depot/file1,v.new &&
     -		mv depot/file1,v.new depot/file1,v &&
     -		printf "@$UTF16@" >>depot/file1,v &&
    ++		#
    ++		# Note that newer Perforce versions started to store files
    ++		# compressed in directories. The case statement handles both
    ++		# old and new layout.
     +		case "$(echo depot/file1*)" in
     +		depot/file1,v)
     +			sed -e "\$d" depot/file1,v >depot/file1,v.new &&
2:  ee5d836b77 ! 2:  f41473d991 ci: update Perforce version to r23.2
    @@ Commit message
         Update our Perforce version from r21.2 to r23.2. Note that the updated
         version is not the newest version. Instead, it is the last version where
         the way that Perforce is being distributed remains the same as in r21.2.
    -    Newer releases stopped distributing p4 and p4d executablesas well as the
    -    macOS archives directly and would thus require more work.
    +    Newer releases stopped distributing p4 and p4d executables as well as
    +    the macOS archives directly and would thus require more work.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
3:  d0a80ba403 = 3:  bf03405db9 t98xx: mark Perforce tests as memory-leak free

base-commit: d19b6cd2dd72dc811f19df4b32c7ed223256c3ee

Comments

Justin Tobler July 31, 2024, 8:50 p.m. UTC | #1
On 24/07/31 12:37PM, Patrick Steinhardt wrote:
> Hi,
> 
> this is the second version of my patch series that improves Perforce
> tests a bit. As with the first version, the main intent is to mark all
> tests as leak free. The other two patches polish the Perforce tests such
> that they work on my machine, which has a more recent version of
> Perforce than what we have been testing with in CI.
> 
> As discussed, despite these patches I consider Perforce to be basically
> unmaintained, and my patches do not change that. But given that I don't
> think we can rip out git-p4(1) just so without any kind of announcement
> up front, I still think that these patches are worth it to help my own
> cause, namely plugging all memory leaks hit by tests in Git.

Agreed that proposing a removal of git-p4(1) is more of a separate issue
here. For now, these patches make sense so we can get closer to making
the tests memory leak free. :)

> 
> Changes compared to v1:
> 
>   - Fix some typos.
> 
>   - Add in-code comments explaining what the compatibility case
>     statements are about.
> 
> Thanks!
> 
> Patrick

From looking at the range diff, this version looks good to me. Thanks

-Justin
Junio C Hamano July 31, 2024, 10:32 p.m. UTC | #2
Justin Tobler <jltobler@gmail.com> writes:

> From looking at the range diff, this version looks good to me. Thanks

Thanks, both.  Perhaps I should mark this for 'next' by now.