Re: Review Request 41778: Proposal: simplify test BUILD files to one target per directory.

2015-12-29 Thread Joshua Cohen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41778/#review112223
---


I'm generally ok with this this. The main benefit of smaller target groups is 
faster test runs, but tbh, I've probably spent more time looking up which 
smaller target has the tests I want to run than I've saved ;).


src/test/python/apache/aurora/client/api/BUILD (line 16)


While we're at it, can we name the test targets to match the directory (so 
`api` in this case)? That way we can use pants target shorthand of `./pants 
test src/test/python/apache/aurora/client/api` rather than having to do, e.g., 
`api:all`


- Joshua Cohen


On Dec. 29, 2015, 9:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41778/
> ---
> 
> (Updated Dec. 29, 2015, 9:39 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> I'm using this file as a proposed convention for test targets in `BUILD` 
> files.  In addition to being less redundant, i find the resulting file much 
> easier to understand.  In the past when refactoring, i can find it 
> nightmarish to go back and bring all the build targets back into line (often 
> repeating myself with imports changed in code).
> 
> If this proposal is accepted, i would like to further propose we collapse our 
> `BUILD` files into one target for all python tests.
> 
> Note that this patch is related to 
> https://issues.apache.org/jira/browse/AURORA-959
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/aurora/client/api/BUILD 
> 2a55cec51324c18debf10a1da93a74043f288a93 
> 
> Diff: https://reviews.apache.org/r/41778/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41778: Proposal: simplify test BUILD files to one target per directory.

2015-12-29 Thread Bill Farner 


> On Dec. 29, 2015, 1:46 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/api/BUILD, line 81
> > 
> >
> > Thinking out loud - i'm guessing this doesn't do what i want and roll 
> > up the tests within the other target.  I suppose this means i would need 
> > one `python_tests` target for the directory and one `target` to roll up the 
> > others?

Checked for myself and it actually does work - `python_test` targets within 
`dependencies` will be executed!


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41778/#review112220
---


On Dec. 29, 2015, 1:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41778/
> ---
> 
> (Updated Dec. 29, 2015, 1:39 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> I'm using this file as a proposed convention for test targets in `BUILD` 
> files.  In addition to being less redundant, i find the resulting file much 
> easier to understand.  In the past when refactoring, i can find it 
> nightmarish to go back and bring all the build targets back into line (often 
> repeating myself with imports changed in code).
> 
> If this proposal is accepted, i would like to further propose we collapse our 
> `BUILD` files into one target for all python tests.
> 
> Note that this patch is related to 
> https://issues.apache.org/jira/browse/AURORA-959
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/aurora/client/api/BUILD 
> 2a55cec51324c18debf10a1da93a74043f288a93 
> 
> Diff: https://reviews.apache.org/r/41778/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41778: Proposal: simplify test BUILD files to one target per directory.

2015-12-29 Thread Bill Farner 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41778/#review112226
---


Thanks for the quick feedback.  Sounds like there's enough momentum for me to 
flesh out the patch more.  Please hold ship-its until i have a more substantial 
version of the patch.

- Bill Farner


On Dec. 29, 2015, 1:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41778/
> ---
> 
> (Updated Dec. 29, 2015, 1:39 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> I'm using this file as a proposed convention for test targets in `BUILD` 
> files.  In addition to being less redundant, i find the resulting file much 
> easier to understand.  In the past when refactoring, i can find it 
> nightmarish to go back and bring all the build targets back into line (often 
> repeating myself with imports changed in code).
> 
> If this proposal is accepted, i would like to further propose we collapse our 
> `BUILD` files into one target for all python tests.
> 
> Note that this patch is related to 
> https://issues.apache.org/jira/browse/AURORA-959
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/aurora/client/api/BUILD 
> 2a55cec51324c18debf10a1da93a74043f288a93 
> 
> Diff: https://reviews.apache.org/r/41778/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41778: Proposal: simplify test BUILD files to one target per directory.

2015-12-29 Thread Bill Farner 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41778/#review112220
---



src/test/python/apache/aurora/client/api/BUILD (line 24)


Thinking out loud - i'm guessing this doesn't do what i want and roll up 
the tests within the other target.  I suppose this means i would need one 
`python_tests` target for the directory and one `target` to roll up the others?


- Bill Farner


On Dec. 29, 2015, 1:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41778/
> ---
> 
> (Updated Dec. 29, 2015, 1:39 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> I'm using this file as a proposed convention for test targets in `BUILD` 
> files.  In addition to being less redundant, i find the resulting file much 
> easier to understand.  In the past when refactoring, i can find it 
> nightmarish to go back and bring all the build targets back into line (often 
> repeating myself with imports changed in code).
> 
> If this proposal is accepted, i would like to further propose we collapse our 
> `BUILD` files into one target for all python tests.
> 
> Note that this patch is related to 
> https://issues.apache.org/jira/browse/AURORA-959
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/aurora/client/api/BUILD 
> 2a55cec51324c18debf10a1da93a74043f288a93 
> 
> Diff: https://reviews.apache.org/r/41778/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41778: Proposal: simplify test BUILD files to one target per directory.

2015-12-29 Thread John Sirois 


> On Dec. 29, 2015, 2:46 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/api/BUILD, line 81
> > 
> >
> > Thinking out loud - i'm guessing this doesn't do what i want and roll 
> > up the tests within the other target.  I suppose this means i would need 
> > one `python_tests` target for the directory and one `target` to roll up the 
> > others?
> 
> Bill Farner wrote:
> Checked for myself and it actually does work - `python_test` targets 
> within `dependencies` will be executed!

Yes, but there is really no need for test meta-targets.  We can use `./pants 
test src/test/python/apache/aurora/client/api:` for all in this dir, `./pants 
test src/test/python/apache/aurora/client/api::` for everything in this dir and 
its children recursively, and we can mix in `--tag=-it` for just unit tests (if 
we add tags={'it'} to integration test targets) or `--tags=+it` for just 
integration tests.


- John


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41778/#review112220
---


On Dec. 29, 2015, 2:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41778/
> ---
> 
> (Updated Dec. 29, 2015, 2:39 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> I'm using this file as a proposed convention for test targets in `BUILD` 
> files.  In addition to being less redundant, i find the resulting file much 
> easier to understand.  In the past when refactoring, i can find it 
> nightmarish to go back and bring all the build targets back into line (often 
> repeating myself with imports changed in code).
> 
> If this proposal is accepted, i would like to further propose we collapse our 
> `BUILD` files into one target for all python tests.
> 
> Note that this patch is related to 
> https://issues.apache.org/jira/browse/AURORA-959
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/aurora/client/api/BUILD 
> 2a55cec51324c18debf10a1da93a74043f288a93 
> 
> Diff: https://reviews.apache.org/r/41778/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 41778: Proposal: simplify test BUILD files to one target per directory.

2015-12-29 Thread Bill Farner 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41778/
---

Review request for Aurora, John Sirois and Zameer Manji.


Repository: aurora


Description
---

I'm using this file as a proposed convention for test targets in `BUILD` files. 
 In addition to being less redundant, i find the resulting file much easier to 
understand.  In the past when refactoring, i can find it nightmarish to go back 
and bring all the build targets back into line (often repeating myself with 
imports changed in code).

If this proposal is accepted, i would like to further propose we collapse our 
`BUILD` files into one target for all python tests.

Note that this patch is related to 
https://issues.apache.org/jira/browse/AURORA-959


Diffs
-

  src/test/python/apache/aurora/client/api/BUILD 
2a55cec51324c18debf10a1da93a74043f288a93 

Diff: https://reviews.apache.org/r/41778/diff/


Testing
---


Thanks,

Bill Farner

Re: Review Request 41778: Proposal: simplify test BUILD files to one target per directory.

2015-12-29 Thread Bill Farner 


> On Dec. 29, 2015, 1:46 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/api/BUILD, line 81
> > 
> >
> > Thinking out loud - i'm guessing this doesn't do what i want and roll 
> > up the tests within the other target.  I suppose this means i would need 
> > one `python_tests` target for the directory and one `target` to roll up the 
> > others?
> 
> Bill Farner wrote:
> Checked for myself and it actually does work - `python_test` targets 
> within `dependencies` will be executed!
> 
> John Sirois wrote:
> Yes, but there is really no need for test meta-targets.  We can use 
> `./pants test src/test/python/apache/aurora/client/api:` for all in this dir, 
> `./pants test src/test/python/apache/aurora/client/api::` for everything in 
> this dir and its children recursively, and we can mix in `--tag=-it` for just 
> unit tests (if we add tags={'it'} to integration test targets) or 
> `--tags=+it` for just integration tests.

Ok, since we're already doing that for jenkins (`src/{main,test}/python::`), i 
will remove the refernces to child targets.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41778/#review112220
---


On Dec. 29, 2015, 1:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41778/
> ---
> 
> (Updated Dec. 29, 2015, 1:39 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> I'm using this file as a proposed convention for test targets in `BUILD` 
> files.  In addition to being less redundant, i find the resulting file much 
> easier to understand.  In the past when refactoring, i can find it 
> nightmarish to go back and bring all the build targets back into line (often 
> repeating myself with imports changed in code).
> 
> If this proposal is accepted, i would like to further propose we collapse our 
> `BUILD` files into one target for all python tests.
> 
> Note that this patch is related to 
> https://issues.apache.org/jira/browse/AURORA-959
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/aurora/client/api/BUILD 
> 2a55cec51324c18debf10a1da93a74043f288a93 
> 
> Diff: https://reviews.apache.org/r/41778/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 41778: Proposal: simplify test BUILD files to one target per directory.

2015-12-29 Thread John Sirois 


> On Dec. 29, 2015, 2:54 p.m., Joshua Cohen wrote:
> > I'm generally ok with this this. The main benefit of smaller target groups 
> > is faster test runs, but tbh, I've probably spent more time looking up 
> > which smaller target has the tests I want to run than I've saved ;).

And - fwiw, the test discovery is pretty fast such that `./pants test :: -- 
-ktest_module_name` is good enough for a medium sized python project like this 
since chroots are now cached.
... except for aurora's current need to --no-fast (sealed in in pants.ini), so 
this is really `./pants test.pytest --fast :: -- -ktest_non_hooked_api` and 
there are more oddities - dup module names + pytest ... but - FWIW.


- John


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41778/#review112223
---


On Dec. 29, 2015, 2:39 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41778/
> ---
> 
> (Updated Dec. 29, 2015, 2:39 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> I'm using this file as a proposed convention for test targets in `BUILD` 
> files.  In addition to being less redundant, i find the resulting file much 
> easier to understand.  In the past when refactoring, i can find it 
> nightmarish to go back and bring all the build targets back into line (often 
> repeating myself with imports changed in code).
> 
> If this proposal is accepted, i would like to further propose we collapse our 
> `BUILD` files into one target for all python tests.
> 
> Note that this patch is related to 
> https://issues.apache.org/jira/browse/AURORA-959
> 
> 
> Diffs
> -
> 
>   src/test/python/apache/aurora/client/api/BUILD 
> 2a55cec51324c18debf10a1da93a74043f288a93 
> 
> Diff: https://reviews.apache.org/r/41778/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>