On 4/26/2017 1:45 PM, Brett Cannon wrote:

On Tue, 25 Apr 2017 at 17:00 Terry Reedy <tjre...@udel.edu <mailto:tjre...@udel.edu>> wrote:

    While I use code coverage to improve automated unittesting, I am opposed
    to turning a usable but limited and sometime faulty tool into a blind
    robotic master that blocks improvements.  The prospect of this being
    done has discouraged me from learning the new system.  (More on 'faulty
    tool' later.)

It should be stated that code coverage is not a blocking status check for merging from our perspective (the **only** required check is that Travis pass with it's test run).

I have the impression that at one time you hoped to make it blocking. If that was wrong, I apologize for misunderstanding. If you have changed your mind, then I am happy.

I am otherwise in favor of both the measurement and report of coverage being improved.

    The temptation to write artificial tests to satisfy an artificial goal
    is real.  Doing so can eat valuable time better used for something else.
       For instance:

          def meth(self, arg):
              mod.inst.meth(arg, True, ob=self, kw='cut')

    Mocking mod.class.meth, calling meth, and checking that the mock is
    called will satisfy the robot, but does not contribute much to the goal
    of providing a language that people can use to solve problems.

My assumption is that there will be a test that meth() does the right thing, mock or not. If we provide an API there should be some test for it; using a mock or something else to do the test is an implementation detail.

My impression is that default mocks have a generic signature, so that merely checking that the mock is called will not catch an invalid call. I presume that one can do better with mocks, and I have with custom mocks I have written, but my point above was that coverage does not care.

     >> If it's not important enough to require tests >> it's not
    important enough to be in Python.  ;)

    Modules in the test package are mostly not tested. ;)


:) But they are at least executed which is what we're really measuring here and I think all Ethan and I are advocating for.

I thought Ethan was advocating for more -- a specific unittest for each line.

E.g. I don't expect test_importlib to be directly responsible for exercising all code in importlib, just that Python's entire test suite exercise importlib as much as possible as a whole.

The advantage for importlib in this respect is that import statements cannot be mocked; only the objects imported, after importlib is finished.

There is lots of interaction between idlelib modules, but I would still like direct tests of each idlelib.xyz with a test_xyz.py. Three years ago, there was no test.test_idle. There now is, and it runs 35 idlelib.idle_test.text_* files. (There are 60 idlelib modules.)

The problem I have with just doing manual testing for things that can easily be covered by a unit test -- directly or indirectly -- is there are technically 85 people who can change CPython today. That means there's potentially 85 different people who can screw up the code ;) .

At the moment, I am the only one pushing idlelib patches, except when it gets included in one of Serhiy's multi-module refactoring patches (and he always nosies me). That skews my view a bit. However, with most of the critical issues fixed, I am anxious to automate what I can of what I now do by hand.

Making sure code is exercised somehow by tests at least minimizes how badly someone like me might mess something thanks to me not realizing I broke the code.

I had not thought about the issue that way. I should add a test_module for each remaining module, import the module, and at least create an instance of every tkinter widget defined therein, and see what other classes could be easily instantiated and what functions easily run.
    Some practical issues with coverage and CodeCov:

    2. Some statements are only intended to run on certain systems, making
    100% coverage impossible unless one carefully puts all system-specific
    code in "if system == 'xyz'" statements and uses system-specific
    .coveragerc files to exclude code for 'other' systems.

True. We could have a discussion as to whether we want to use Coverage.py's pragmas ... I'm sure we could discuss things with Ned Batchelder if we needed some functionality in coverage.py for our needs).

Let's skip this for now.

    3. Some tests required extended resources.  Statements that are only
    covered by such tests will be seen as uncovered when coverage is run on
    a system lacking the resources.  As far as I know, all non-Windows
    buildbots and CodeCov are run on systems lacking the 'gui' resource.  So
    patches to gui code will be seen as uncovered.

I view 100% coverage as aspirational, not attainable. But if we want an attainable goal, what should we aim for? We're at 83.44% now

On what system? I suspect that Tkinter, ttk, turtle, and IDLE GUI-dependent tests make at least a 2% difference on GUI Windows versus no-GUI *nix.

  we could
say that 80% is something we never want to drop below and be done with it. We could up it to 85% or 90% in recognizing that there is more testing to do but that we will never cover all Python code (all of this is configurable in Codecov, hence why I'm asking).

Since I think we actually are at 85%, and certainly will be when I add minimal easy tests for the rest of IDLELIB, I think 90% would be a reasonable goal.

One way to increase coverage is to push a bit harder on fulfilling the 'test needed' stage. Theoretically, every substantive (behavior-changing) patch should start with a test that fails. Since PRs are separate from the main repository and can be patched separately, a PR could start with a test that should immediately fail but should pass before merging. It would be nice if the test runner knew to only run the new test and not the entire suite. It would be even nicer if it know that initial failure is success. Is there at least a 'New Test' label on PRs?

    4. As I explained in a post on the core-workflow list, IDLE needs the
    following added to the 'exclude_lines' item of .coveragerc.
          .*# htest #
          if not _utest:

These additions would remove, I think, at least 400 lines from the uncovered category. Both only occur in idlelib.

--
Terry Jan Reedy
_______________________________________________
python-committers mailing list
python-committers@python.org
https://mail.python.org/mailman/listinfo/python-committers
Code of Conduct: https://www.python.org/psf/codeofconduct/

Reply via email to