Re: [PATCH v6 4/5] dir_iterator: refactor state machine model

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Apr 2, 2017, 05:35 UTC
Message-ID: <235ce437-9f87-e309-cec4-8214bb76b47f@alum.mit.edu>
In-Reply-To: <CAEA2_RJuxw+sZ0CABCfTMnY+Wy5KD2totG76FDWfpH1Uht2taw@mail.gmail.com>

On 04/02/2017 06:46 AM, Daniel Ferreira (theiostream) wrote:

> Gah, I just realized I failed to correct refs/files-backend.c's
> behavior and kept 0 instead of DIR_ITERATOR_PRE_ORDER_TRAVERSAL as its
> flag. I'll correct this on a v7, but I'll wait for the rest of your
> reviews before sending that revision.

Even if I make that change, a lot of tests fail, both when I run the tests against your commit 4/5 and when I run them against 5/5.

I tested by applying your patch series to the current upstream master branch. When I apply the patches, there is one conflict (related to #includes), in entry.c. Maybe the failure is because you are working from a different branch point. What commit are you branching your series off of?

You should definitely run the full test suite against each of your commits before you submit. One convenient way to do so is with my tool `git-test` [1], which runs the tests and keeps track of their results to avoid testing the same commit over and over again:

    # One-time setup (adjust the "-j" values based on what's
    # fastest on your computer):
    git test add "make -j8 && make -j16 test"

    # Run tests against a range of commits:
    git test run master..mybranch

Even better is to run the tests in a separate linked worktree, so that you can continue working in your main repository while the tests are running. The `git-test` README explains how.

It also speeds things up (and gives better output) if you use `prove` to run the tests, and run the tests on a ramdisk if possible. To do so, I include the following lines in my `config.mak`:

    TMP := /var/tmp
    ROOT := $(TMP)/git-tests-$(shell git rev-parse --show-toplevel |
sha1sum | head -c10)
    DEFAULT_TEST_TARGET = prove
    GIT_TEST_OPTS = -q --root=$(ROOT) --tee
    GIT_PROVE_OPTS = --timer --jobs 16 --state=fresh,hot,slow,save

(On my system `/var/tmp` is a ramdisk.)

Finally, when I submit a patch series, I usually also push a copy to my GitHub fork of the project and include a reference to that branch in my cover letter. That makes it easier for casual readers to fetch the code and/or look at it online, and also makes it 100% clear what branch point I've chosen. This is by no means obligatory but I find it helpful.

There doesn't seem to be much point reviewing broken code, so I'll wait for your feedback and/or fix.

Michael

[1] https://github.com/mhagger/git-test