houseabsolute / p5-Test-Vars

Detects unused variables in unit tests
https://metacpan.org/pod/Test::Vars
Other
6 stars 10 forks source link

all_vars_ok should find files dynamically #5

Open doherty opened 10 years ago

doherty commented 10 years ago

In doherty/Dist-Zilla-Plugin-Test-UnusedVars#5, @karenetheridge pointed out that using the static MANIFEST means that all_vars_ok doesn't test all the files. In particular, it misses any which were generated at build time. They aren't listed in the MANIFEST, because MANIFEST is about listing the files that should be included in the tarball. If you listed a file that wasn't there yet, the build tools would think their kit was incomplete.

MANIFEST isn't intended for this purpose. Instead, you should dynamically search for the files you're going to test.

Compare with how Test::Compile or Test::Version do it.

gfx commented 10 years ago

The idea sounds good, but I have some requests:

doherty commented 10 years ago

Could you explain why on both counts? It seems like doing the unsurprising thing by default is a better choice. Also, I'm not sure I see the harm in the additional (common) dependencies for an author test.