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

phpunit support? #62

Closed
eridal opened this issue Jan 22, 2014 · 5 comments
Closed

phpunit support? #62

eridal opened this issue Jan 22, 2014 · 5 comments

Comments

@eridal
Copy link

eridal commented Jan 22, 2014

I saw the test files are written using some odd phpt syntax.
Is there a plan for adding phpunit support? how much coverage this project have?

@jkufner
Copy link
Contributor

jkufner commented Jan 23, 2014

This "odd syntax" is what PHP itself uses for testing. And I have to say, it is way more readable and practical than PHPUnit and similar testing frameworks.

Your patch is very nice example why PHPUnit-like tests are bad. Your code looks like many other tests I saw, nothing bad at the first sight, but...

First, "109 additions and 40 deletions." ... the same feature, double the code. That is a bad sign.

Second, comparing the basic test I noticed few things.

  • Tests are not numbered. Order of execution is important for tests. Once basic test fail, there is no need to look at more complicated tests. I guess PHPUnit has some dependency-related features, but numbering is better, because tests can act as a lectures built from examples.
  • Expected values must be escaped to fit into strings and arrays. That is annoying, because it is harder to read. And this escaping must be done by a human. That is even more annoying. With PHPT you only have to copy-paste verified output, and sometimes replace changing parts with some wildcard.
  • Asserts must be written. And human must think which asserts are required. A plain text diff done by PHPT checks all that with no effort. Really. Once your debug dumps produce nice result, you are done.

Take a look at PHPT -- http://qa.php.net/write-test.php and give it a few minutes of exploring.

Point is, that tests are not only to verify that code works. Actually that is one of minor reasons why tests exist. Good tests are awsome documentation -- working examples ready to run and experiment with. PHPT tests are also perfect as playground during development. You can write code, let it dump whatever is interesting, and once code works, little cleanup of debug dumps, copy-paste to expected section and feature is ready, including tests as an almost free side product.

And the most important, tests must be extremely easy to write. Otherwise nobody would write them.

@eridal
Copy link
Author

eridal commented Jan 23, 2014

Hey, thanks for your feedback.

It really wasn't my idea to start a phpunit-phpt war, and I can truly understand that my patch could have been interpreted that way -- please excuse me if I was rude.

Regarding your comments:
I had included two features: (1) is phpunit support, and (2) is a small test case. To be fair you should say "32 additions, 31 deletions" for the same test. All the other additions are required to provide (a) no hardcoded configuration, (b) dynamic PDO driver tests.

Second,

  • Running tests in the same order may lead to hidden bugs; that's why there are testrunner which shuffle them. This allows to ensure that each test is idempotent and thus not rely on the state left behind of other tests.
  • The escaping thing is more a issue with the expected assert value than with the syntax itself (heredocs may help in such scenario, although I find them not nice)
  • phpunit provides a good assert library which can handle more complex cases than plain text diff.. and it provides more insight about why it failed (like diff between arrays)

I'm not saying that phpt tests are bad; they are a good match for limited environments (such a php internals) but the defacto standard for php projects is phpunit.

@jkufner
Copy link
Contributor

jkufner commented Jan 23, 2014

... please excuse me if I was rude.

No excuse needed. ;-)

To be fair you should say "32 additions, 31 deletions" for the same test.

Not really. The test you replaced has 10 lines of code. The rest, the "expected output", is written automatically and programmer only have to confirm that it is correct (by copy-pasting it into the file). That is the trick which makes such tests much easier to write.

Running tests in the same order may lead to hidden bugs ...

PHPT executes each test in new PHP instance (new process for each test). So if database is prepared and cleaned up correctly, there is no way they could interfere. Am I right? Does PHPUnit also execute each test in new process?

The escaping thing ...
phpunit provides a good assert library ...

With plain text diff there is no need for assert library in the first place. In some cases it may be better to use assert, but such case can be solved using better format of test output.

I'm not saying that phpt tests are bad; they are a good match for limited environments (such a php internals) but the defacto standard for php projects is phpunit.

150 lines of your commit says otherwise ;) It is ok to say that something is bad. I'm using PHPT in few other projects and I really want to know, whether there is a reason why I should not, or where are limits and shortcommings of such approach. Or if PHPUnit can provide some killer features which are worh writing more ugly tests.

@lichtner
Copy link
Collaborator

lichtner commented Feb 2, 2014

@eridal I appreciate your effort but sorry I agree with @jkufner. I don't see any advantages. On the other hand I see many drawbacks:

  1. phpt is less code
  2. phpt is more legible

Try to imagine someone who doesn't know phpt neither phpunit. What is easier to read for newbie? IMHO phpt.

On the other hand your reason for phpunit "defacto standard for php projects" is only
one of the typical rhetological fallacies : Appeal to popular belief - Claiming something is true, because the majority of people believe (use) it.

@eridal
Copy link
Author

eridal commented Feb 4, 2014

I understand and respect your decision, however I think it's not in line with current modern development standards; test runner integration, coverage reports, profiling, dependency analysis are some benefits phpunit can bring (just on top of my mind) to the project.

Anyways, I'm not here to enlighten anybody.. so let's keep this closed as it's.

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

No branches or pull requests

3 participants