Fixes issues #150, #151, #156 and #160 #153

arcivanov · 2015-07-27T06:31:37Z

Forking option in coverage and unittest is now deprecated (removed in unittest since never made public)
Coverage will always fork
Unittest will always fork unless is already forked in coverage
utils.fork_process is now used in both to capture exit code, returned value and possible exception (with traceback) from a forked process
Module reloading is removed since it doesn't ever work when modules keep state and rely on intermodule dependencies
This should be a problem since coverage and unittests run in different forks and will reinitialize the modules anyway

Closes #150, closes #151

arcivanov · 2015-07-27T10:00:16Z

ionelmc/python-tblib#7 fixed that should deal with 2.6 compatibility once accepted

arcivanov · 2015-07-27T12:02:48Z

You're absolutely correct, let me fix this.

arcivanov · 2015-07-27T12:04:03Z

Is it a first runtime dependency or I'm missing something? @mriehl

mriehl · 2015-07-27T13:39:32Z

Well it's required if someone pip installs pybuilder, so it should be runtime.
PR looks good, do you want to merge and release it? I can grand you write access on PyPI if you want to do it.

arcivanov · 2015-07-27T14:14:23Z

I think merge is fine and I think we can release this, although I'm thinking of at least a couple more improvements (fairly minor) at this point. If you grant me PyPi access it would be great as well!

mriehl · 2015-07-27T15:59:46Z

I'll need your PyPI user name to grant you permissions - you can send it via email if you don't want to disclose it publicly.

arcivanov · 2015-07-27T16:01:19Z

It's the same - arcivanov. Thanks!

mriehl · 2015-07-27T16:09:46Z

Right, thanks! Should work now.

arcivanov · 2015-07-30T07:49:43Z

@mriehl @locolupo this is done and ready for review. See commends in #156.

Forking option in coverage and unittest is now deprecated (removed in unittest since never made public) Coverage will always fork Unittest will always fork unless is already forked in coverage utils.fork_process is now used in both to capture exit code, returned value and possible exception (with traceback) from a forked process Module reloading is removed since it doesn't ever work when modules keep state and rely on intermodule dependencies This should be a problem since coverage and unittests run in different forks and will reinitialize the modules anyway Closes pybuilder#150, closes pybuilder#151

https://hg.python.org/cpython-fullhistory/diff/2230/Python/import.c Cases that are now also handled are: .pyw, module.so, module.dll, .o, module.o

mriehl · 2015-07-31T07:07:24Z

src/main/python/pybuilder/errors.py

@@ -56,12 +54,14 @@ def __init__(self, first, second=None):
                CircularTaskDependencyException, self).__init__("Circular task dependency detected between %s and %s",
                                                                first,
                                                                second)
+        else:
+            super(CircularTaskDependencyException, self).__init__(first)


Why is this required? e.G. how can we have a cycle but only one node is known?

By not calling an init we don't call a super into exceptions. This makes the exception unpicklable.

PS: Python serialization sucks worse than Java 😄

I reworked this block

mriehl · 2015-07-31T07:16:47Z

There's a problem when building a new project (pyb --start-project and accept all defaults) without sources or tests:

PyBuilder version 0.10.63
Build started at 2015-07-31 09:15:06
------------------------------------------------------------
[INFO]  Building tmp version 1.0.dev0
[INFO]  Executing build in /tmp
[INFO]  Going to execute task publish
[INFO]  Running unit tests
[INFO]  Executing unit tests from Python modules in /tmp/src/unittest/python
[WARN]  No unit tests executed.
[INFO]  All unit tests passed.
[INFO]  Building distribution in /tmp/target/dist/tmp-1.0.dev0
[INFO]  Copying scripts to /tmp/target/dist/tmp-1.0.dev0/scripts
[INFO]  Writing setup.py as /tmp/target/dist/tmp-1.0.dev0/setup.py
[INFO]  Collecting coverage information
[INFO]  Running unit tests
[INFO]  Executing unit tests from Python modules in /tmp/src/unittest/python
[WARN]  No unit tests executed.
[INFO]  All unit tests passed.
[WARN]  Overall coverage is below 70%:  0%
Coverage.py warning: No data was collected.
Traceback (most recent call last):
  File "/usr/lib/python2.7/multiprocessing/queues.py", line 264, in _feed
    send(obj)
PicklingError: Can't pickle <class 'pybuilder.errors.BuildFailedException'>: it's not the same object as pybuilder.errors.BuildFailedException

Looks like some reimport related problem with pickling being too picky about types when adding errors to the queue.

Also note the build hangs afterwards due to the process not terminating normally - we need to fix this before pushing out a release.

mriehl · 2015-07-31T07:18:02Z

Code LGTM aside from the comment above.

This contribution is fantastic - thank you. I never gave it much thought before you brought up the issue but of course self-bootstrapping while running coverage is totally nontrivial...

arcivanov · 2015-08-01T13:43:55Z

@mriehl Thanks for tracking down pyb --start-project - I reworked the fork exception handling logic.

Fix "syntax error/compile" warning Rework message queue mechanism in forking making it more robust Cover the new exception handling in forking with more tests

branch tracking is added html report generation is added in $dir_reports/<execution_prefix>_html/ modules to be covered are explicitly passed to report generator ensure modules have no duplicates

arcivanov · 2015-08-02T14:26:06Z

@mriehl I added the #160. If you could perform the final review so that I can merge it would be great.

mriehl · 2015-08-02T15:21:38Z

I think it's good to go, LGTM!

arcivanov · 2015-08-02T15:25:00Z

Super, thanks!

Fixes issues #150, #151, #156 and #160

coveralls · 2017-04-27T16:25:22Z

Coverage increased (+27.1%) to 100.0% when pulling ab6488f on arcivanov:issue_150_151 into c9ecba2 on pybuilder:master.

arcivanov added the in progress label Jul 27, 2015

arcivanov added code-review-required enhancement ready and removed in progress labels Jul 27, 2015

arcivanov mentioned this pull request Jul 29, 2015

Great coverage disparity while building PyB #156

Closed

arcivanov self-assigned this Jul 29, 2015

arcivanov added blocked to-be-verified-by-reporter and removed ready labels Jul 29, 2015

arcivanov force-pushed the issue_150_151 branch 5 times, most recently from 8102ae8 to 0fe9b55 Compare July 30, 2015 07:02

arcivanov added ready bug and removed blocked to-be-verified-by-reporter labels Jul 30, 2015

arcivanov changed the title ~~Fixes issues #150 and #151~~ Fixes issues #150, #151 and #156 Jul 30, 2015

arcivanov force-pushed the issue_150_151 branch from 0fe9b55 to f85cffe Compare July 30, 2015 10:05

Remove tblib initialization and put it lazyily in the fork_process

277dbea

arcivanov force-pushed the issue_150_151 branch from 5755762 to 277dbea Compare July 30, 2015 13:09

Bring module inspection into compliance with Python import logic

63407ec

https://hg.python.org/cpython-fullhistory/diff/2230/Python/import.c Cases that are now also handled are: .pyw, module.so, module.dll, .o, module.o

mriehl reviewed Jul 31, 2015
View reviewed changes

arcivanov force-pushed the issue_150_151 branch from 11dca24 to 93e2790 Compare August 1, 2015 13:41

arcivanov force-pushed the issue_150_151 branch 5 times, most recently from 63bc4f5 to 920abf1 Compare August 1, 2015 14:21

Redo circular exception messaging

880da14

Fix "syntax error/compile" warning Rework message queue mechanism in forking making it more robust Cover the new exception handling in forking with more tests

arcivanov force-pushed the issue_150_151 branch 3 times, most recently from 41f0662 to 8b02606 Compare August 2, 2015 02:28

arcivanov changed the title ~~Fixes issues #150, #151 and #156~~ Fixes issues #150, #151, #156 and #160 Aug 2, 2015

fixes issue pybuilder#160

ab6488f

branch tracking is added html report generation is added in $dir_reports/<execution_prefix>_html/ modules to be covered are explicitly passed to report generator ensure modules have no duplicates

arcivanov force-pushed the issue_150_151 branch from 8b02606 to ab6488f Compare August 2, 2015 02:32

arcivanov added a commit that referenced this pull request Aug 2, 2015

Merge pull request #153 from arcivanov/issue_150_151

7b09367

Fixes issues #150, #151, #156 and #160

arcivanov merged commit 7b09367 into pybuilder:master Aug 2, 2015

arcivanov removed the ready label Aug 2, 2015

arcivanov deleted the issue_150_151 branch April 27, 2017 18:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes issues #150, #151, #156 and #160 #153

Fixes issues #150, #151, #156 and #160 #153

arcivanov commented Jul 27, 2015

arcivanov commented Jul 27, 2015

arcivanov commented Jul 27, 2015

arcivanov commented Jul 27, 2015

mriehl commented Jul 27, 2015

arcivanov commented Jul 27, 2015

mriehl commented Jul 27, 2015

arcivanov commented Jul 27, 2015

mriehl commented Jul 27, 2015

arcivanov commented Jul 30, 2015

mriehl Jul 31, 2015

arcivanov Jul 31, 2015

arcivanov Aug 1, 2015

mriehl commented Jul 31, 2015

mriehl commented Jul 31, 2015

arcivanov commented Aug 1, 2015

arcivanov commented Aug 2, 2015

mriehl commented Aug 2, 2015

arcivanov commented Aug 2, 2015

coveralls commented Apr 27, 2017

Fixes issues #150, #151, #156 and #160 #153

Fixes issues #150, #151, #156 and #160 #153

Conversation

arcivanov commented Jul 27, 2015

arcivanov commented Jul 27, 2015

arcivanov commented Jul 27, 2015

arcivanov commented Jul 27, 2015

mriehl commented Jul 27, 2015

arcivanov commented Jul 27, 2015

mriehl commented Jul 27, 2015

arcivanov commented Jul 27, 2015

mriehl commented Jul 27, 2015

arcivanov commented Jul 30, 2015

mriehl Jul 31, 2015

Choose a reason for hiding this comment

arcivanov Jul 31, 2015

Choose a reason for hiding this comment

arcivanov Aug 1, 2015

Choose a reason for hiding this comment

mriehl commented Jul 31, 2015

mriehl commented Jul 31, 2015

arcivanov commented Aug 1, 2015

arcivanov commented Aug 2, 2015

mriehl commented Aug 2, 2015

arcivanov commented Aug 2, 2015

coveralls commented Apr 27, 2017