Skip to content

Comments

Mention individual test file target in the test Makefile#6871

Merged
WalterBright merged 1 commit intodlang:masterfrom
wilzbach:makefile_goody
Jun 13, 2017
Merged

Mention individual test file target in the test Makefile#6871
WalterBright merged 1 commit intodlang:masterfrom
wilzbach:makefile_goody

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Jun 8, 2017

I guess everyone has found his favorite solution over the years, but why not adding a good "out-of-the-box" solution?

I re-open the Makefile as imho that's cleaner than duplicating the build commands (AFAIK there's no nice way in Make to force re-execution of dependencies).

@braddr
Copy link
Member

braddr commented Jun 8, 2017

Already exists: make resultsdir + testname including path +.out
ex: make test_results/runnable/arrayop.d.out

@wilzbach
Copy link
Contributor Author

wilzbach commented Jun 8, 2017

Already exists: make resultsdir + testname including path +.out
ex: make test_results/runnable/arrayop.d.out

I know (after all this target gets called), but (1) .test behaves similarly to the Phobos .test target (i.e. it always gets executed) and (2) there's now a nice entry at the beginning of the Makefile, s.t. people don't need to search.

@CyberShadow
Copy link
Member

The comment is welcome, but I don't think we need the extra Makefile rules. I've observed that the more of those we have, the more painful everything becomes, so we should look for opportunities to shrink our makefiles, not grow them.

@wilzbach wilzbach force-pushed the makefile_goody branch 2 times, most recently from 66fd584 to 6516ac4 Compare June 10, 2017 02:01
@wilzbach wilzbach changed the title Add a convenient way to run tests individually Mention individual test file target in the test Makefile Jun 10, 2017
@wilzbach
Copy link
Contributor Author

The comment is welcome, but I don't think we need the extra Makefile rules. I've observed that the more of those we have, the more painful everything becomes, so we should look for opportunities to shrink our makefiles, not grow them.

Hmm ok. I rebased then. As the target section started to grow, I used plain-text lines to help the reader to immediately visually distinguish between the sections.

@wilzbach wilzbach added the Review:Trivial typos, formatting, comments label Jun 10, 2017
@CyberShadow
Copy link
Member

I still don't understand why the new rules are needed.

  1. The behavior of rm-ing the target - why? There should already be a dependency upon the built dmd, so it shouldn't make sense to need to force a rebuild of the target.

  2. Instead of adding new rules to save typing a few characters, my suggestion was to improve the documentation and specify how to run individual rules.

@wilzbach
Copy link
Contributor Author

I still don't understand why the new rules are needed.
The behavior of rm-ing the target - why? There should already be a dependency upon the built dmd, so it shouldn't make sense to need to force a rebuild of the target.

Yes convenience. Shell completion works nicely and then you just need to add .test ;-)

Instead of adding new rules to save typing a few characters, my suggestion was to improve the documentation and specify how to run individual rules.

Didn't you see that I already changed this PR?

@wilzbach
Copy link
Contributor Author

Didn't you see that I already changed this PR?

Somehow my push didn't went through. Sorry about this. The .test targets are gone now.

@CyberShadow
Copy link
Member

Yes convenience. Shell completion works nicely and then you just need to add .test ;-)

Shell completion should be able to partially complete a common prefix, so test_results/ should be complete-able as well.

Didn't you see that I already changed this PR?

Yep, looks good.

@WalterBright WalterBright merged commit 64e0aec into dlang:master Jun 13, 2017
@wilzbach wilzbach deleted the makefile_goody branch December 19, 2017 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review:Trivial typos, formatting, comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants