Only run tests for modules affected by a PR

[dhermes]

Inspired by googleapis/google-cloud-node#1579

This may require building a directed graph (based on direct imports) and traversing the graph. We could use networkx for this, or maybe pylint already has tooling for dependency graphs.

[tseaver]

Seems like overkill: all 2160 unit tests run in under 4 seconds on my machine. Now, if we could prune the system tests, that would be a win (but only after merge, of course).

[dhermes]

Yes "Only run tests" was meant to include all tests (maybe unit tests not relevant as you point out)

[theacodes]

@dhermes in python-docs-samples we make nox gather a list of changed files and then filter the list of samples to test based on that. Maybe some of that code could be useful to you?

[dhermes]

Ha, some bubble gum there for sure. When I tried to use the TRAVIS_COMMIT env. var. long ago (like almost 2 whole years) it was not consistently populated.

[theacodes]

Yep, that's why we have a fallback that uses HEAD.

On Thu, Sep 8, 2016, 3:38 PM Danny Hermes notifications@github.com wrote:

Ha, some bubble gum there for sure. When I tried to use the TRAVIS_COMMIT
env. var. long ago (like almost 2 whole years) it was not consistently
populated.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#2277 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAPUc_oO1T7O24aFfDs6O2oYicXiKb0Nks5qoI5ygaJpZM4J4bOm
.

[dhermes]

I wanted to summarize how TRAVIS_* env. vars. are used in what @jonparrott linked to. Essentially, they are only used to detect if on Travis (${TRAVIS} == true) and to get a list of changed files in the build:

if ${TRAVIS_PULL_REQUEST} == false
  git show --pretty=format: --name-only ${TRAVIS_COMMIT}
else
  try
    git diff --name-only ${TRAVIS_COMMIT} ${TRAVIS_BRANCH}
  fallback
    HEAD_COMMIT = git rev-parse HEAD
    git diff --name-only ${HEAD_COMMIT} ${TRAVIS_BRANCH}
  end
end

[daspecster]

All system tests should be run before a release is tagged. I think the danger of some change affecting another module in some way would outweigh the speed savings.

[dhermes]

That's why I suggested building the dependency graph.

[daspecster]

I'm skeptical that a dependency graph would catch everything all the time. Just seems like adding risk?

I may just be paranoid.

[dhermes]

Yes you are definitely paranoid. If code A doesn't depend on code B (whether directly, or indirectly, as traced along the dependency graph) then code B can be completely changed without ever making an impact on the performance of code A (with lots of caveats, e.g. code B can't delete the OS).

[daspecster]

Yeah I understand. I withdraw my concerns based on your confidence.

Just make sure whatever makes the graph works with namespaced packages 😉!

[tseaver]

Relying on a third-party dependency-graph analysis tool does add risks: if it has bugs, we could end up not testing code which actually is affected by a change.

[dhermes]

Worth noting that TRAVIS_COMMIT_RANGE is worthless sometimes, but AFAICT this only happens when rebases / force-pushes occur. Otherwise, the TRAVIS_COMMIT_RANGE looks like ${START}...${END} where END is just the HEAD commit in the branch that was pushed and START is the last value of START in a Travis build for that branch.

[pkch]

Did you close this because the potential benefit doesn't seem to be worth the effort, or because this problem was solved through some other PRs? We're dealing with the same question in our project, so I was hoping to borrow ideas =)

[dhermes]

@pkch We have mostly solved it by examining the files in the new commits.

I am trying to make this into a generic helper in http://ci-diff-helper.readthedocs.io/en/latest/ if you want to take a look.