pybuilder / pybuilder

Software build automation tool for Python.
https://pybuilder.io
Apache License 2.0
1.71k stars 247 forks source link

Reimplement/rethink "analyze" task #201

Open arcivanov opened 9 years ago

arcivanov commented 9 years ago

Currently analyze task makes very little sense, is confusing and produces non-deterministic and counterintuitive results.

If you have unit tests, cram tests and integration tests and don't specify analyze as a top-level task, analyze still runs but only analyzes unit tests ignoring cram and integration tests. If analyze is specified as a top-level task then analyze runs after all the tests.

If the coverage, for example, is combined for unit and integration tests, and then checked for 100% combined coverage, then running pyb publish will fail because analyze, not being explicitly invoked, runs after unit tests only, where as the same code with pyb analyze publish will pass all the tests.

I would propose either:

  1. Getting rid of analyze altogether and performing all necessary checks in verify making it a responsibility of individual plugins to bind to that verify task and performing aggregate analysis for all tasks executed by the reactor.
  2. Replacing analyze with analyze_sources, analyze_unit_tests, analyze_integration_tests etc making it the responsibility of those plugins individually.
  3. Something else

@mriehl thoughts?

mriehl commented 9 years ago

I like being able to differentiate between "check for functional defects" and "check for warts and potential bugs / style issues". But you're right that it's confusing... I think 2. would be too low level but maybe if we set the dependencies of those tasks the right way it could work.

The issue I see with 1. is that I won't be able to have a customizable execution order for unit tests / linters. Say I only want to unit test if flake8 has no complaints (or the other way around). Then clearly those need to be separate tasks.

Maybe another possibility is having publish (the default task out of the box) depend on analyze and analyze depend on verify? This is what I usually want (no point in getting "line too long" errors when my tests are red) but this would make it difficult to get it working the other way around.

arcivanov commented 9 years ago

Well, there are two approaches:

Another thing to consider is maybe we will simply remove analyze, unit and integration test and coverage default tasks once I'm done with #188. Then each plugin individually adds either analyze_xxx task or adds to a common analyze task.

Do we want to have flexibility to either fail-on-error and fail-on-end, like Maven does?

mriehl commented 9 years ago

I think we should always fail fast because most things (like upload or integration tests) don't make sense when the build is failed.

Getting rid of analyze and verify with #188 sounds good - this would give us flexibility to either call the top-level tasks that flak8 etc reverse-depend on, or set a custom order with default_task. Right now we can't run "just flake8" since the analyze namespace is cluttered by many plugins.

arcivanov commented 9 years ago

@mriehl I naturally meant fail-on-end before result-generating tasks such as publish, install or upload.

Alternative would also be to add an attribute to "@task(fault_tolerant=False)" and add it to publishers, which would make publishers etc intolerant of previous failures while the other tasks can proceed.

I need to sleep on this :smile: