Skip to content

Conversation

@sinbad
Copy link
Contributor

@sinbad sinbad commented Aug 6, 2015

Spotted while working on other things that assert_pointer wasn't a good test for fetch, since it only validates that the git commit objects have the right pointer data, not that the content exists locally.

Added assert_local_object and refute_local_object instead & used in fetch tests.

assert_pointer only checked that a pointer existed, not that the content was present. Add assert_local_object and refute_local_object to test helpers
@sinbad sinbad force-pushed the fetch-test-local-storage branch from 99360f1 to 5e654f2 Compare August 6, 2015 11:13
@technoweenie
Copy link
Contributor

Ah yeah, good call 👍

technoweenie added a commit that referenced this pull request Aug 6, 2015
Fetch tests should test local storage
@technoweenie technoweenie merged commit af42c0c into git-lfs:master Aug 6, 2015
@sinbad sinbad deleted the fetch-test-local-storage branch August 6, 2015 16:17
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 18, 2024
In commit 5e654f2 in PR git-lfs#565 a pair
of test assertion functions were added to the forerunner of our
current t/testhelpers.sh shell library.  These assert_local_object()
and refute_local_object() functions check for the presence or absence
of a file in the object cache maintained by the Git LFS client in
a local repository.

To perform these checks, the functions capture the output of the
"git lfs env" command and parse the contents of the LocalMediaDir
line, which reports the full path to the Git LFS object cache
location.  To retrieve the path, the functions ignore the first 14
characters of the line, as that corresponds to the length of the
LocalMediaDir field name (13 characters) plus one character in order
to account for the equals sign which follows the field name.

Later PRs have added three other assertion functions that follow
the same design.  The delete_local_object() function was added in
commit 97434fe of PR git-lfs#742 to help test
the "git lfs fetch" command's --prune option, the corrupt_local_object()
function was added in commit 4b0f50e
of PR git-lfs#2082 to help test the detection of corrupted local objects
during push operations, and most recently, the assert_remote_object()
function was added in commit 9bae8eb
of PR git-lfs#5905 to improve our tests of the SSH object transfer protocol
for Git LFS.

All of these functions retrieve the object cache location by ignoring
the first 14 characters from the LocalMediaDir line in the output of
the "git lfs env" command.  However, the refute_local_object() function
contains a hint of an alternative approach to parsing this line's data.

A local "regex" variable is defined in the refute_local_object()
function, which matches the LocalMediaDir field name and equals sign
and captures the subsequent object cache path value.  Although this
"regex" variable was included when the function was first introduced,
it has never been used, and does not appear in any of the other similar
functions.

While reviewing PR git-lfs#5905, larsxschneider suggested an even simpler
option than using a regular expression to extract the object cache
path from the LocalMediaDir line.  Rather than asking the Bash shell
to start its parameter expansion at a fixed offset of 14 characters
into the string, we can define a pattern which matches the leading
LocalMediaDir field name and equals sign and specify that the shell
should remove that portion of the string during parameter expansion.
See also the discussion in this review comment from PR git-lfs#5905:

  git-lfs#5905 (comment)

In addition to these changes, we can remove the definition of the
"regex" variable from the refute_local_object() function, as it
remains unused.

Co-authored-by: Lars Schneider <[email protected]>
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 this pull request may close these issues.

2 participants