Test-More / Test2-Harness

Alternative to Test::Harness
Other
23 stars 26 forks source link

yath should default things like --blib based on -d ./blib #147

Closed toddr closed 4 years ago

toddr commented 4 years ago

I've just done a quick parse of yath test --help.

I suggest that --blib, --tlib, --lib, and --dev-lib default to not injecting into @INC if the directory they would inject is not a valid directory path (symlinks are ok?)

exodist commented 4 years ago

Complications with this: 1) Sometimes yath is run with a chdir option telling it to change dirs before running tests, relative paths, including lib/blib need to have the chdir path accounted for. 2) the projects command specially needs to account for chdir 3) Some paths are added as env-vars tot he runner, the job, or both, which get inserted into PERL5LIB.

Anyway, if we do this, Test2/Harness/Runner/job.pm needs both env_vars() and cli_includes() updated, and possibly others.

I am not considering this a blocker for major_refactor merge/release. If someone submits a PR I like I will merge it.

atoomic commented 4 years ago

137 mainly fixes part of the concerns, by using full path.

In truth the complaint is having by default lib and blib on. The counter argument is that we need them more often that we do not.

Maybe we can consider doing a -d check on the directory and throw it away only if it s a full path. [starting with /]

It looks that blib and lib should be mutually exclusive. But this is another discussion. Would be nice to have -b and -l aliases, but in truth as long as the default is on, this is useless.

atoomic commented 4 years ago

I suggest this change 091ff154681055e9f178e9642d808b455b60e09b to close this issue

haarg commented 4 years ago

It isn't obvious what the actual problem you are trying to solve with this ticket is.

toddr commented 4 years ago

The suggestion is to not inject INC paths that do not exist since the default is to have -b and -l on by default

haarg commented 4 years ago

That is the suggestion. That is not the problem. What is the problem?

toddr commented 4 years ago

For me changing the defaults is problematic. If nobody else cares, then maybe there's not a problem. We are in the middle of a major re-factor, so defining this new behavior doesn't seem like a bad design choice IMO.

haarg commented 4 years ago

I don't really like having lib and blib included by default either, but trying to sometimes add them and sometimes not just doesn't really seem like it's solving any real problems.

On another note, blib and lib definitely shouldn't be mutually exclusive. When working on the perl code for XS modules, it's very useful to compile the XS once into blib, but then modify and test against the pm files in lib.

exodist commented 4 years ago

Being reminded that the order in which they are added matters, and significantly, and the fact it can change is the first thing of this ticket that makes me bump it from "meh" to "important".

I think 'lib' should be added by default, the times you do not want it are pretty rare. I also think adding blib by default makes sense (for the case haarg mentioned). By default I want to be able to run yath in an XS modules dist without specifying lib/blib and have it find the compiled xs in blib, but use lib for the perl.

So for default I want it to add lib, then blib, that order. However I want it to be trivial to change the order or negate either/both paths for the case where you just want lib or blib, or want them in the reverse order (which sounds super rare, but someone will probably want to do it).

I still am not comfortable with the "adding only if they exist" feature, but at the same time I guess it is effectively a no-op. I feel like what makes more sense is to die or warn if a path that is specified does not exist, though that is too noisy for any that are added by default.

I am going to play with the options a bit and see what I can work out. Ideally it will look like this:

  1. include lib and blib, in that order by default, after any -I's you add
  2. -l and -b short options will be added, if either is specified then the default will only be to add the ones specified, and in the order specified, IE -l means just lib, -b means just blib, -bl means adding blib then lib, -lb means adding lib then blib
  3. If you specify -b or -l then yath will die if either one is not present, cause if you added them at the cli you are telling yath you expect them to be present. If you want to specify one and do not want to be strict use -Ilib or similar.
  4. I am ok with the default lib/blib (no -l or -b on the cli) not being added if the paths do not exist, I have a weird feeling about it like there is something I know of that would make this a bad idea, but I cannot put my finger on it, and logic tells me it is effectively a no-op.

Summary, add -l and -b options, preserve their order along with -I order, active by default lib, then blib, after -I's when they are only in by default. Strict when manually specified, added only when they exist when they are only there because of defaults.

I fear this may be over-complicated, but I absolutely want the lib+blib default, but also want simple options for people who have different use cases.

toddr commented 4 years ago

How ever you want to handle this (including closing this case) @exodist , I'm fine with it.

exodist commented 4 years ago

See the commit above, that is my include system overhaul. I am considering this ticket "solved" as "won't fix". If you want more details read the commit.