BF (codename "happy travis"): trying to address lints etc to make Travis green again #702

yarikoptic · 2017-11-28T01:39:21Z

Some minor RFings did creep in, hopefully I didn't change any logic (tests will show ;) ).


          RF: primarily flake8 lints + minor RF to reduce duplication in PATHEXT

I did keep some "bare" except with catch all Exception: , while tried to disable flake8 complaints where clearly all exceptions are to be catched


          BF(PY26): {} -> {0}, i.e. explicit index for .format()


          BF: crazy tests ppl pass an object for status... uff -- catch TypeErr…

…or too


          RF: last of flake8 fails - avoid using temp variable in a test

codecov-io · 2017-11-28T14:01:20Z

Codecov Report

Merging #702 into master will increase coverage by 1.51%.
The diff coverage is 91.93%.

@@            Coverage Diff             @@
##           master     #702      +/-   ##
==========================================
+ Coverage   92.83%   94.35%   +1.51%     
==========================================
  Files          61       61              
  Lines       10152    10136      -16     
==========================================
+ Hits         9425     9564     +139     
+ Misses        727      572     -155

Impacted Files	Coverage Δ
git/test/test_submodule.py	`99.23% <ø> (+1.71%)`	⬆️
git/repo/base.py	`95.96% <0%> (+0.67%)`	⬆️
git/refs/log.py	`93.84% <0%> (ø)`	⬆️
git/diff.py	`99.11% <100%> (ø)`	⬆️
git/test/test_git.py	`98.8% <100%> (+0.01%)`	⬆️
git/test/test_commit.py	`100% <100%> (ø)`	⬆️
git/compat.py	`65.03% <100%> (+0.21%)`	⬆️
git/exc.py	`96.42% <100%> (ø)`	⬆️
git/test/test_util.py	`98.52% <100%> (+10.29%)`	⬆️
git/test/lib/helper.py	`91.86% <50%> (+4.06%)`	⬆️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a96030...f48d087. Read the comment docs.


          RF(+BF?): refactor hooks creation in a test, and may be make it compa…

…t with windows


          ENH: add appveyor recipe to establish rdesktop login into the test box


          ENH: also report where on sh, and echo msg when entering on_finish

yarikoptic · 2017-11-28T15:57:13Z

Travis is happy now, fighting with windows

yarikoptic · 2017-11-28T16:43:57Z

my question would be -- did hooks ever work on windows??? or tests were testing only that they "fail to work"? ;)

yarikoptic · 2017-11-28T16:50:43Z

the last succesful build on appveyor was https://ci.appveyor.com/project/Byron/gitpython/build/1.0.161 -- at that point there where no tests for succesful hooks execution. So AFAIK they never worked, and I am offloading that for future to #703 . In this one I will skip those tests tests to establish a reliable baseline for now


          RF: no "need" for custom shebang on windows since just does not work


          Disable (but keep for future uses commented out) hook into appveyor s…

…ession

ankostis · 2017-11-28T16:53:50Z

Kudos to tour efforts and happy fighting ;-)

Regarding hooks, at the time of #519 and for some months later all remaining test-cases were passing on Windows, e.g. https://ci.appveyor.com/project/ankostis/gitpython/build/1.0.433
You may read #525 where i explain the trick to artificially pass Appveyor, and use it to mark also the hooks-TCs as permanently failing.

BTW you may check if your recent efforts have fixed some of those long disabled TCs.


          RF(TST): skip all tests dealing with hooks on windows


          BF(WIN): use where instead of which while looking for git

yarikoptic · 2017-11-28T17:07:21Z

@ankostis -- join the "fun"! but I think/hope I am "done"

hooks -- as of https://ci.appveyor.com/project/ankostis/gitpython/build/1.0.433 you reference, there were no test for testing successful execution of a hook on windows .

Thanks for the pointer to HIDE_WINDOWS_KNOWN_ERRORS -- I will use it instead of plain is_win (just will wait for tests to finish or fail to get a baseline ;))

As for the 'check' -- someone should just check if there is a similar to travis allow_failures, if so -- then there could be a dedicated matrix run which would do that for us, without requiring manual messing around... but that later -- for now just want it to get green.

yarikoptic · 2017-11-28T17:09:33Z

But in the future (after we are green again), it would be nice if no PR was merged if tests fail...


          RF: use HIDE_WINDOWS_KNOWN_ERRORS instead of is_win to skip hooks tests


          BF(WIN): where could report multiple hits, so choose first

yarikoptic · 2017-11-28T19:49:28Z

woohoo -- some of the windows environments are green now! rebuilding to see if some spurious one didn't creep in. any immediate clue about what could be wrong in those anaconda windows ones @ankostis ?

yarikoptic · 2017-11-28T20:42:08Z

ok, @Byron , @ankostis et al, it is ready for the review. I will leave the appveyor crusade for later/someone else ATM (note a handy addition to .appveyor.yml) - at least some environments are in clear there as well, and travis is all green. Would be nice to establish the baseline of passing CI in master.

ankostis · 2017-11-28T20:44:01Z

I think it is not anaconda to blame but rather Cygwin's Git!
Since git-2.8+, I've seen huge problems with joining local paths, causing most of the 16 failures in the last 2 environments.

For instance in git-2.8+ it doesn't even work the command pip install git+https://github.com/foo/bar.git.
I believe it is this problem: http://lists-archives.com/git/894111-cygwin-git-cannot-install-python-packages-with-pip.html

yarikoptic · 2017-11-28T22:45:42Z

Yikes

ankostis · 2017-11-29T22:20:42Z

pip install git+http scheme failure on cygwin reported as pypa/pip#4895.

yarikoptic · 2017-11-30T01:07:33Z

Well... would be nice to get it reviewed/merged at this stage of success so that at least some baseline is established back again, and then tackle cygwin in a more dedicated PR ;)


          to keep travis busy - adding myself to AUTHORS

hugovk

👍

It's important to get at least one CI green.

hugovk · 2017-12-05T08:45:06Z

git/test/lib/helper.py

@@ -139,7 +139,7 @@ def repo_creator(self):
            try:
                try:
                    return func(self, rw_repo)
-                except:
+                except:  # noqa E722


How about except Exception: like elsewhere?

We could. But at this out-most level of handling execution, I thought to remain as "greedy" as possible. May be someone (among used dependencies) still manages to raise non-Exception "exceptions" up the chain from somewhere/somehow -- so I thought to remain catching them as well. In the other places it was more "specific" to not expect some complete weird stuff being thrown up. But if you insist, we could have it Exception here as well

hugovk · 2017-12-05T08:45:06Z

git/test/lib/helper.py

@@ -296,7 +296,7 @@ def remote_repo_creator(self):
                    with cwd(rw_repo.working_dir):
                        try:
                            return func(self, rw_repo, rw_daemon_repo)
-                        except:
+                        except:  # noqa E722


How about except Exception: like elsewhere?

Byron · 2017-12-11T10:54:46Z

@yarikoptic Thanks so much for hanging into this! Especially your crusade to get the windows CI green is particularly brave!! @ankostis certainly has a tale to tell there, too!
I am merging this as it already is getting us some steps towards a somewhat sane Christmas release :D! Full disclaimer: I am on vacation now to the end of the year, and intend to get my inbox empty and spend the time it takes.
@yarikoptic @ankostis I understand I was the bottleneck for the upcoming release, and can't even promise betterment. However, if you are not yet able to cut a release on your own, but would like to, I am happily assigning you the rights needed and add your GPG signature keys to the ones officially endorsed by this repository. Just let me know, and we will make it happen.

yarikoptic · 2017-12-11T14:49:52Z

Thanks @Byron , I hope you will have a nice (and productive in all regards) vacation!
Here is my primary key, (public portion) should be available from public servers

pub   rsa4096/A2DE235062DA33FA 2010-06-07 Yaroslav Halchenko (Yarik) <[email protected]>
 Primary key fingerprint: C5B9 05F0 E8D9 FD96 68FF  366F A2DE 2350 62DA 33FA

I usually (for other projects) "tag -s" releases locally, push the tags, create release out of them on github

I am yarikoptic on pypi


          Add Yarikoptic to allowed release keys

> pub rsa4096/A2DE235062DA33FA 2010-06-07 Yaroslav Halchenko (Yarik) <[email protected]> > Primary key fingerprint: C5B9 05F0 E8D9 FD96 68FF 366F A2DE 2350 62DA 33FA As per #702 (comment)

Byron · 2017-12-11T15:31:06Z

@yarikoptic Thanks - I added you to the list of maintainers on PyPi and your key to the release-verification-key.asc, so you should be able to make your own signed releases from now on. Just to stay in training, I would like to do v2.1.8 though :).

yarikoptic · 2017-12-11T15:41:43Z

oh yeah, I would prefer not to release unless I really have to ;) so I am ok to stay in training for as long as it could be ;-)

yarikoptic added 4 commits Nov 28, 2017

RF: primarily flake8 lints + minor RF to reduce duplication in PATHEXT

6ee08fc

I did keep some "bare" except with catch all Exception: , while tried to disable flake8 complaints where clearly all exceptions are to be catched

BF(PY26): {} -> {0}, i.e. explicit index for .format()

Loading status checks…

086af07

BF: crazy tests ppl pass an object for status... uff -- catch TypeErr…

Loading status checks…

e1aea3a

…or too

RF: last of flake8 fails - avoid using temp variable in a test

Loading status checks…

c352dba

RF(+BF?): refactor hooks creation in a test, and may be make it compa…

Loading status checks…

42e89cc

…t with windows

yarikoptic force-pushed the yarikoptic:bf-happy-travis branch from 9a3fd7a to 42e89cc Nov 28, 2017

yarikoptic added 2 commits Nov 28, 2017

ENH: add appveyor recipe to establish rdesktop login into the test box

Loading status checks…

d723148

ENH: also report where on sh, and echo msg when entering on_finish

Loading status checks…

dcfe242

yarikoptic mentioned this pull request Nov 28, 2017

Hooks are not executed on windows correctly #703

Open

yarikoptic added 2 commits Nov 28, 2017

RF: no "need" for custom shebang on windows since just does not work

0a67f25

Disable (but keep for future uses commented out) hook into appveyor s…

6253625

…ession

yarikoptic added 2 commits Nov 28, 2017

RF(TST): skip all tests dealing with hooks on windows

91b9bc4

BF(WIN): use where instead of which while looking for git

Loading status checks…

b4459ca

yarikoptic added 2 commits Nov 28, 2017

RF: use HIDE_WINDOWS_KNOWN_ERRORS instead of is_win to skip hooks tests

cc34077

BF(WIN): where could report multiple hits, so choose first

Loading status checks…

4d851a6

yarikoptic force-pushed the yarikoptic:bf-happy-travis branch from add36d5 to 4d851a6 Nov 28, 2017

yarikoptic mentioned this pull request Nov 28, 2017

BF: process included files before the rest #700

Merged

3 of 3 tasks complete

ankostis added the tag.Windows label Nov 28, 2017

to keep travis busy - adding myself to AUTHORS

Loading status checks…

f48d087

hugovk approved these changes Dec 5, 2017

View changes

hugovk mentioned this pull request Dec 5, 2017

Fix flake8 to fix Travis CI #705

Closed

Byron merged commit a14277e into gitpython-developers:master Dec 11, 2017
1 of 2 checks passed

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

yarikoptic deleted the yarikoptic:bf-happy-travis branch Dec 15, 2017

gitpython-developers / GitPython

BF (codename "happy travis"): trying to address lints etc to make Travis green again #702

BF (codename "happy travis"): trying to address lints etc to make Travis green again #702

yarikoptic commented Nov 28, 2017

codecov-io commented Nov 28, 2017 •

edited

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

ankostis commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

ankostis commented Nov 28, 2017 •

edited

yarikoptic commented Nov 28, 2017

ankostis commented Nov 29, 2017

yarikoptic commented Nov 30, 2017

hugovk left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Byron commented Dec 11, 2017

yarikoptic commented Dec 11, 2017

Byron commented Dec 11, 2017

yarikoptic commented Dec 11, 2017

gitpython-developers / GitPython

Join GitHub today

BF (codename "happy travis"): trying to address lints etc to make Travis green again #702

BF (codename "happy travis"): trying to address lints etc to make Travis green again #702

Conversation

yarikoptic commented Nov 28, 2017

codecov-io commented Nov 28, 2017 • edited

Codecov Report

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

ankostis commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

yarikoptic commented Nov 28, 2017

ankostis commented Nov 28, 2017 • edited

yarikoptic commented Nov 28, 2017

ankostis commented Nov 29, 2017

yarikoptic commented Nov 30, 2017

hugovk left a comment

This comment has been minimized.

hugovk Dec 5, 2017 Contributor

This comment has been minimized.

yarikoptic Dec 5, 2017 Author Collaborator

This comment has been minimized.

hugovk Dec 5, 2017 Contributor

Byron commented Dec 11, 2017

yarikoptic commented Dec 11, 2017

Byron commented Dec 11, 2017

yarikoptic commented Dec 11, 2017

codecov-io commented Nov 28, 2017 •

edited

ankostis commented Nov 28, 2017 •

edited

hugovk Dec 5, 2017
Contributor

yarikoptic Dec 5, 2017
Author Collaborator

hugovk Dec 5, 2017
Contributor