Implement update call when the object is "up to date" #871

[t89]

In the current state the update() method of RemoteProgress is not called, if the progress terminates with ** = [up to date]**.

With this implementation, the method gets called once, containing all useful pieces of information. By passing current_count = 1 and max_count = 1 we assure that any percentage calculation terminates with 100%.

TODO:

• Please check, if the op_code = 0 makes sense in this case
• I opted for a more future proof, yet slower solution in the elif:
  • to trade durability for performance, use .startswith(' = [up to date]')

[jeking3]

Need some sort of test that exercises this and proves the update occurs?

[t89]

Sure. When I cloned your repository I used tox to make an initial testrun and the tests failed right out of the box. I looked at the closed (implemented) pull requests and saw that most of them fail the tests, so I didn't bother to investitage the test cases any further.

I'm relatively new to Python unit tests. Can you point me in the right direction? I do appreciate it.

[jeking3]

I am able to do a clean "git clone" followed by "tox" and it tests py27, py36, py37 locally and successfully.

[t89]

test.log
I ran $ git pull main master, installed the (test-)requirements.txt using pip install -r and ran tox / nosetests both terminate with the same result:

FAILED (SKIP=41, errors=10, failures=3)

I can reproduce this two machines. Suggestions?

Running $ nosetests yields the same result.

[t89]

I also investigated your testfile regarding the updated method and I know too little about GitPython's internals to write the test. Like I mentioned in my initial pull-request, I kindly ask the maintainer of the repo to insert the correct op_code, etc. pp.

[t89]

Any feedback on this topic? "It works on my machine" isn't really helpful at all. A part of my current project relies on this functionality and I would like to put in the time to make this pull-request easily mergeable for you.

To write a proper test for this pull-request it would be great to know...

• ...a hint on where to start to fix the unit-tests of GitPython (see log in earlier post)
• ...which op_code you would like me to send to the update() method when the project status is = [up to date]
• ...how you would implement a test inside your project which tests, if update() is called when _parse_progress_line() is passed Git's up-to-date-status.
  • Should I create a local flag inside the test class to know when update should have been called? This seems messy and maybe there is a pythonic way of handling this more elegantly.

Thanks for your time.

[jeking3]

It may be that you have a very old or very new version of git? What does git --version say?

[Byron]

Hi all, and sorry for the late reply. May I also suggest to have a look at the travis file, as it performs a few commands that prepare the local clone to be sufficient to run the tests. Yes, the tests are not isolated and brutally rely on the local clone for read-only operations. At this point I am not sure anymore if my machine still qualifies to run them correctly, however, travis should still be green and can be triggered with every push.

…

On Mon 13. May 2019 at 17:40, James E. King III ***@***.***> wrote: It may be that you have a very old or very new version of git? What does git --version say? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#872?email_source=notifications&email_token=AAAPRBQGYSKOLFL3U62DKPTPVFLCDA5CNFSM4HLIOYRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVIDQQQ#issuecomment-491796546>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAPRBUE4Z4SOOB3NNLCRDDPVFLCDANCNFSM4HLIOYRA> .

[jeking3]

You may also be running into a bug in the unit test framework - I don't think it can handle anything other than the master branch without errors. I haven't reported it yet (I actually forgot I ran into it months ago), but if you tried to run tox on a branch other than master, that might be why. Perhaps the previous comment has guidance to work around that?

[t89]

Wanted to let you know that I still intend to fix this issue, at the moment I don't have the sparetime thoug. Current workaround is freezing the GitPython dependency oin the current version and overriding the method with my own.

I'll let you know, once I come back to this.

[Byron]

As part of spring cleaning, I am closing this PR as it was opened more than half a year ago while still being open due to unresolved or remarks.

If this is a mistake or any other action should rather be taken, please let me know in the comments or create a new PR.
Thanks for your understanding and the work that undoubtedly went into this.