Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Comments

Update Tester and Docs#134

Open
dakujem wants to merge 1 commit intoTharos:develop from
dakujem:feat/tester
Open

Update Tester and Docs #134
dakujem wants to merge 1 commit intoTharos:develop from
dakujem:feat/tester

Conversation

@dakujem
Copy link
Contributor

@dakujem dakujem commented Sep 7, 2018

Changes:

  • Updated Tester to version v2 to enable for -C switch (see below)
  • Added section to readme about testing
  • Added 2 Composer commands for testing:
    • composer test (run all tests)
    • composer test-local (run all tests using local PHP configuration, via the -C Tester switch)

Maybe some info about contribution can be added by you.

Also, what about the following sentence ??

"This library is no longer actively developed." ...

Copy link
Contributor Author

dakujem commented Sep 7, 2018

Oh... okay. PHP 5.4 and 5.5 issue.

Copy link
Collaborator

janpecha commented Sep 7, 2018

I plan to drop PHP 5 support and update Tester in Lean Mapper 4.x.

For now IMHO would be fine add only contributing.md with instructions - how to create branch for PR, how to test changes (with list of required extensions), etc. Something like https://nette.org/en/contributing.

Composer commands

IMHO it's unnecessary because everybody needs something else - set another php.ini, run to coverage, Windows vs Unix issues, etc.

"This library is no longer actively developed." ...

I think we can remove it.

Copy link
Contributor Author

dakujem commented Sep 7, 2018

That composer command test-local is very flexible and will work for most local environments (Win, Linux and Mac, provided they have composer installed) - all those extensions are pretty much standard for any local development. It is also super easy and even the lazy folks can run it. I personally prefer that to looking for a way to run the tests, then failing 5 times because of misspelled pathname, studying how nette Tester in particular is run (for newcommers) or similar issues.

Personally, I would only keep the local command as default for composer test, but I included that other one because I thought you were using the unix config.

Copy link
Collaborator

janpecha commented Sep 9, 2018

I thought about it and I have some notes.

  • vendor/bin/tester -C doesn't work in Tester 1.7 so test-local is useless for now
  • I don't want to have hardcoded vendor/bin/tester -c php-unix.ini in composer.json because in Tester 1.7 we need flexibility:
    • we need specify right php.ini for current environment
    • sometimes we need change PHP binary (-p php, -p php5.6 or something else)
    • a lot of another posibilities (watch mode,...)
  • newcomers and lazy people must always read manual and make minimal 3 steps:
    1. install required extensions (or check if are installed)
    2. run composer install
    3. run Tester - there are 2 options: composer test or vendor/bin/tester <some arguments>
  • composer test isn't flexibile in Tester 1.7 and is useless in Tester 2.x because there aren't big diferrences between composer test and simple vendor/bin/tester -C
  • good contributing.md is better than one command line shortcut

So I think contributing manual is good idea but we don't need composer test.

Copy link
Contributor Author

dakujem commented Sep 9, 2018
edited
Loading

I agree with Tester v1.7 it does not bring the utility I had in mind and with upgrade to Tester v2 you lose support for legacy PHP. Though I think it is a good idea to just have composer test once you decide to move forward. It's just so easy to type composer test instead of path/to/my/vendor/some-binary some-options-why-uppercase-ffs.

Copy link
Collaborator

It would be great split this PR on 2 parts:

  1. manual for contributors
  2. changes in composer.json

We can add contributors manual in separate PR and this PR can wait for LM 4.x. What do you thing?

Copy link
Contributor Author

dakujem commented Sep 14, 2018

@janpecha Good idea. How can i do that? Or will you?

Copy link
Collaborator

@dakujem I created #136. It's ok for you?

@janpecha janpecha added this to the Version 4.0.0 milestone Sep 14, 2018
Copy link
Contributor Author

dakujem commented Sep 14, 2018

I like it. Let's keep this one open until v4 (or close it, it's up to you, I don't mind).

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Version 4.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /