Fix exlucde_target_regexp breakage in test-changed and --files option breakage in changed with diffspec

Review Request #4321 — Created Oct. 17, 2016 and submitted

yujiec
pants
3977, 3981, 3982
pants-reviews
kwlzn, mateor, stuhood

This review addresses 2 issues:
1. Currently, on v1 engine, exclude_target_regexp does not have effect in test-changed (and also compile-changed since they have same base class).
2. In "changed" task, if both --diffspec and --files options are given, no output is printed.

This review fixes the above 2 issues and adds test coverage. I also add "ensure_engine" decorator to some tests in changed_integration.py which are currently only tested on v1 engine.

https://travis-ci.org/pantsbuild/pants/builds/168473439

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
  1. thanks for tackling these Yujie! a couple of questions below.

  2. looking at this diff and the test just above this one (which appears to set itself up identically but uses --changed-since vs --changed-diffspec), I'd expect their outputs to be the same - yet they aren't. why is that?

    1. so for changed-since, the corresponding git command is "git diff --name-only [revision]", for changed-diffspec the git command is "git diff-tree -r [commit sha]"
      in "git diff" case, the output is difference between specified revision and the current repo.
      in "git diff-tree" case, the output is compared between given sha and its parent.

      https://git-scm.com/docs/git-diff-tree

    2. aha - thanks!

  3. was this meant to be temporary?

    1. Oops...
      Thank you so much for catching this!

  4. 
      
  1. I am kind of rubber stamping the coverage b/c I have to run. I will test the next rc against my repros as it gets released. Thanks for closing the issues for us!

  2. 
      
Review request changed

Status: Closed (submitted)

Change Summary:

Merged in https://github.com/pantsbuild/pants/commit/6b34ffab020036fdd85543440415d97b5f4395b6.
Thanks Kris and Mateo!

Loading...