Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for git sparse checkout #122

Merged
merged 3 commits into from
Aug 30, 2019
Merged

Add support for git sparse checkout #122

merged 3 commits into from
Aug 30, 2019

Conversation

gold2718
Copy link
Collaborator

@gold2718 gold2718 commented Jun 25, 2019

Add support for git sparse checkout

Added a new, optional external property, sparse. sparse points to a file (relative to external repository local directory) with git sparse checkout patterns (e.g., directory names to include).
If the sparse keyword is included, enable sparse checkout for that repository and use git read-tree to perform the sparse checkout.

Added a new test for sparse checkout. The test is a repo with two externals which are both tag2 of the simple_ext repo with one of them configured for a sparse checkout. The test makes sure that both externals have the correct files.

Updated the documentation for the externals configuration file.

User interface changes?: Yes
New sparse keyword is optional with a blank default value which preserves current functionality.

Fixes: #120

Testing:
test removed: None
unit tests: All passed
system tests: All passed, including new test for sparse feature
manual testing: Several manual tests of NASA test cases.

tclune and others added 2 commits June 21, 2019 10:27
Config for repo now has an optional entry 'sparse' which only has an
effect for git repositories.  The value is the name of the file
(relative to the parent repo) to be used for sparsification.

* Updated tests to reflect new config key.

  Note: No tests were added for Git sparse checokout.  Existing tests
  were simply updated to ensure that a field entry for sparse was
  available in all relevant tests.
@gold2718 gold2718 requested a review from billsacks June 25, 2019 21:53
@gold2718 gold2718 self-assigned this Jun 25, 2019
@jedwards4b
Copy link
Collaborator

@gold2718 please fix travis-ci tests

@tclune
Copy link
Collaborator

tclune commented Jun 28, 2019

Note - I was hoping to take a stab at this over the weekend. I'm still not sure how to do such a test in a way that does not involve creating and destroying repositories. Presumably if I look at the existing tests, I'll either see how things are mocked or get more comfortable with a heavy weight test.

@gold2718
Copy link
Collaborator Author

@tclune,
There is a test for sparse checkout in the current PR.
You can create and destroy repos in tests but you can also use the existing repos in test/repos.
For instance, for the new test I wrote, I added a file and tag to test/repos/simple-ext.git

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 88.456% when pulling 6c6ef9f on gold2718:sparse into a48558d on ESMCI:master.

Copy link
Member

@billsacks billsacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a cursory look, looks good. (Sorry for the delay: I was on vacation last week.)

@tclune
Copy link
Collaborator

tclune commented Aug 30, 2019

I had a quiet day and thought I would return to this issue. I presume it has not been merged because of the decrease in coverage, and have investigated. The few new lines of code that are not exercised are print statements activated when verbose is on. I tried modifying the existing test to use verbose_args but that causes coverage to fail - presumably because I'm not properly handling the extra stuff that then goes to STDOUT.

======================================================================
FAIL: test_container_sparse (test_sys_checkout.TestSysCheckout)
Verify that 'full' container with simple subrepo
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_sys_checkout.py", line 1393, in test_container_sparse
    self._check_container_sparse_post_checkout(overall, tree)
  File "test_sys_checkout.py", line 913, in _check_container_sparse_post_checkout
    self._check_simple_tag_ok(tree)
  File "test_sys_checkout.py", line 676, in _check_simple_tag_ok
    self._check_generic_ok_clean_required(tree, name)
  File "test_sys_checkout.py", line 641, in _check_generic_ok_clean_required
    self.assertEqual(tree[name].sync_state, ExternalStatus.STATUS_OK)
AssertionError: u'e' != u' '

I tried to follow the examples from the other tests, but those are all using verbose_args after the initial clone. What is the correct course of action here?

Note - I'm happy to add additional tests, but until I understand this issue, none will increase the code coverage to the best of my understanding.

@gold2718
Copy link
Collaborator Author

@tclune, I have been meaning to try and figure out what is wrong with our test coverage. I agree that something is messed up. For instance, when I added the test for sparse checkout, the coverage went down. I think it is not worth further holding this PR for as I will not be able to look into this for at least another month.

@gold2718 gold2718 merged commit 34fbf55 into ESMCI:master Aug 30, 2019
@gold2718 gold2718 deleted the sparse branch February 11, 2022 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Git sparse checkout
5 participants