Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chdir to $ROOT before running commands #86

Closed
wants to merge 3 commits into from
Closed

chdir to $ROOT before running commands #86

wants to merge 3 commits into from

Conversation

oalders
Copy link
Contributor

@oalders oalders commented Oct 4, 2017

This fixes Perl::Critic spell checks, which look for a stopwords file
that might be a file relative to the root directory.

Fixes #85

@autarch
Copy link
Member

autarch commented Oct 4, 2017

Rebasing off master should fix the CI failures.

This fixes Perl::Critic spell checks, which look for a stopwords file
that might be a file relative to the root directory.
@oalders
Copy link
Contributor Author

oalders commented Oct 4, 2017

I rebased, but that one commit I was missing shouldn't be the problem. It looks to me like t/lib is not being included by the call to prove on Travis.

@autarch
Copy link
Member

autarch commented Oct 4, 2017

Master is passing, so I'm inclined to blame your changes ;)

I'm guessing that it has to do with things like writing use lib 't/lib' relying on the process having a specific current directory.

After introducing the directory change to $ROOT before running commands,
t/Basic.t broke.  The issue seems to be that the command is run in a
different process.  Once the directory change happens, the relative lib
"t/lib" can no longer be found.  In this case, just using FindBin is not
enough to fix the problem.

We could just pass "-I t/lib" as an argument to prove, but that would be
an extra argument to remember.  We could also add it to .proverc, which
means we can get a passing test without the extra argument at the
command line.  This would fix the CI builds.  However, this still does
not fix "dzil test".  This is because "dzil test" runs "make test" and
bypasses prove entirely.  So we need a universal fix which does not rely
on the fact that prove will be used to run the tests.

Messing with PERL5LIB and changing relative paths to absolute is enough
to get these tests in a passing state.

This fix is mostly borrowed from
https://metacpan.org/source/LEONT/Test-Harness-3.39/lib/App/Prove.pm#L586
https://metacpan.org/source/LEONT/Test-Harness-3.39/lib/TAP/Harness.pm#L69
https://metacpan.org/source/LEONT/Test-Harness-3.39/lib/TAP/Parser/SourceHandler/Perl.pm#L162

Thanks to Kent Fredric for working through this with me and basically
just giving me the answers I needed.
@oalders
Copy link
Contributor Author

oalders commented Oct 5, 2017

This is passing now, even if the solution is kind of nasty. You have any better ideas? I'm getting some local failures on dzil test --all, but it might just be my setup. It would be worthwhile checking that this actually works before merging.

If this is ok, it might be a good idea to squash when merging so that the test failure from the first commit doesn't mess with git bisect.

@autarch
Copy link
Member

autarch commented Oct 7, 2017

I merged this from the CLI. Thanks!

@autarch autarch closed this Oct 7, 2017
@oalders
Copy link
Contributor Author

oalders commented Oct 10, 2017

Thanks @autarch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Perl::Critic spelling checks fail when tidyall runs from a subdir
2 participants