Skip to content

Allow using ci-phpunit-test directly from vendor path - #274

Merged
kenjis merged 5 commits into
kenjis:masterfrom
joshkel:use-from-vendor
Nov 6, 2019
Merged

Allow using ci-phpunit-test directly from vendor path#274
kenjis merged 5 commits into
kenjis:masterfrom
joshkel:use-from-vendor

Conversation

@joshkel

@joshkel joshkel commented Nov 4, 2018

Copy link
Copy Markdown

This avoids having to clutter an application's directory with a copy of ci-phpunit-test.

To implement this, I added a CI_PHPUNIT_TESTPATH PHP constant that gives the location of ci-phpunit-test. It defaults to using application/tests/_ci_phpunit_test if that's present (so the default behavior is the same as previous versions. However, if that directory isn't present, then it looks for a vendor folder next to the application folder.

Is this general approach okay? If so, I can update this PR to update documentation and (if appropriate) to update the Composer version of the installer script.

See these comments under #171.

Thanks!

This avoids having to clutter an application's directory with a copy of
ci-phpunit-test.

See comments under kenjis#171.
@coveralls

coveralls commented Nov 4, 2018

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-16.2%) to 56.624% when pulling abeb30d on joshkel:use-from-vendor into e38a140 on kenjis:master.

Exclude from code coverage:  This is test suite bootstrap code, so we
know it's executed, but because it's bootstrap code, it runs outside of
any coverage tracking.
@joshkel

joshkel commented Dec 2, 2018

Copy link
Copy Markdown
Author

I updated the PR to fix a couple of bugs and to keep the code coverage from decreasing (as requested by CONTRIBUTING.md).

I improved the code coverage by perhaps cheating: I had to add additional path handling to CIPHPUnitTest::init, and CIPHPUnitTest::init always executes, but it isn't tracked by code coverage, so the code coverage percentage decreased. I annotated CIPHPUnitTest::init to exclude it from code coverage; since it's test suite bootstrap code, we know it's executed, but because it's bootstrap code, it runs outside of any coverage tracking.

If you'd prefer that it instead remained tracked (as a reminder/placeholder to perhaps someday test it with mocking) and prefer that I find some other way to improve coverage, please let me know.

@ytetsuro

Copy link
Copy Markdown
Contributor

I think it is a problem that can be solved by using a symbolic link, but what kind of benefits can the user get?

$ cd application/tests
$ rm -rf _ci_phpunit_test
$ ln -s ../../vendor/kenjis/ci-phpunit-test/application/tests/_ci_phpunit_test/ .

@joshkel

joshkel commented Mar 1, 2019

Copy link
Copy Markdown
Author

@ytetsuro The biggest benefit would be getting third-party code out of a project's repository - that makes it easier to separate the developers' own code from third-party dependencies, keeps the repo smaller, etc.

It may also make upgrading to a new version of ci-phpunit-repo easier.

I hadn't thought about using symlinks; thank you for the suggestion. However, it would be nice to have an officially supported solution, and symlinks are not straightforward to enable and use for Windows developers.

Updating a working copy from a local installation of ci-phpunit-test to
the Composer-ized one may leave a local, mostly empty _ci_phpunit_test
directory.  Checking for the actual file is more reliable.

@kenjis kenjis left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kenjis
kenjis merged commit 88a49ae into kenjis:master Nov 6, 2019
kenjis added a commit that referenced this pull request Feb 18, 2021
Allow using ci-phpunit-test directly from vendor path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants