Skip to content
This repository was archived by the owner on Aug 15, 2024. It is now read-only.

Updated simplest app scenario (HelloWorld)#90

Closed
lavturo wants to merge 4 commits intophp:masterfrom
lavturo:fix_issue5
Closed

Updated simplest app scenario (HelloWorld)#90
lavturo wants to merge 4 commits intophp:masterfrom
lavturo:fix_issue5

Conversation

@lavturo
Copy link
Contributor

@lavturo lavturo commented Dec 20, 2019

@cmb69 @dalehhirt Please look over the changes. Overall, there are not too many big changes, just changes that were enough to get the HelloWorld scenario to work.

If you want specifics:

-CliPhpUnitTestCaseRunner.java was preventing pftt2 from running since it kept giving an error about php.ini not found.

-PhpUnitSourceTestPack.java had some issues finding some tests since pftt2 has all the paths with a forward-slash instead of back-slash.

-hello_world.groovy was implemented to run the simplest scenario. Kind of gives an idea of how other app scenarios should be structured (wordpress.groovy is probably a better one to reference)

-DOMDocument.php was giving an error regarding assertEquals function not being compatible with another assertEquals*. Updated for the time being. When/if we upgrade PHPUnit, this shouldn't be needed anymore. It is temporary for this function to work.

*Full error message

[Fatal error: Declaration of PHPUnit_Framework_Comparator_DOMDocument::assertEquals($expected, $actual, $delta = 0, $canonicalize = false, $ignoreCase = false) must be compatible with PHPUnit_Framework_Comparator_Object::assertEquals($expected, $actual, $delta = 0, $canonicalize = false, $ignoreCase = false, array &$processed = Array) in C:\pftt2\cache\util\PEAR\pear\PHPUnit\Framework\Comparator\DOMDocument.php on line 115]

Copy link
Contributor

@dalehhirt dalehhirt left a comment

Choose a reason for hiding this comment

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

Can you leave a link to the Hello World test file?

@lavturo
Copy link
Contributor Author

lavturo commented Jan 8, 2020

If you wanted to test this place the entire folder from HelloWorld.zip into pftt2/cache. Otherwise, if you were just looking for the test file, it is in there as well.

@cmb69
Copy link
Member

cmb69 commented Jan 9, 2020

Nice @lavturo, thanks! A next step might be to turn that into a server test (i.e. remove the print statement from the test case, and have a minimal hello_world.php script which prints).

Also, I wonder where to put the test file for future usage.

@lavturo
Copy link
Contributor Author

lavturo commented Jan 10, 2020

@cmb69 @dalehhirt Could you guys check this again? I have updated it to use phpunit-5 for now. There were some workarounds I implemented (mostly in HostEnvUtil.java), but it should work for the time being. I have only checked it with the HelloWorld app scenario and running a regular test with opcache enabled/disabled and the changes seem to be fine.

I was also able to remove a decent amount of files from the cache folder as well since pftt2 will not need those files anymore to run phpunit. However, keep in mind that phpunit-5 only supports 5.6, 7.0, 7.1. So if we were to do more app testing, we would have to change it get the appropriate phpunit (which right now it seems phpunit-7 is the only other one we need to get to support up to 7.4).

I just want some feedback about how it is currently being implemented. Adjustments can be made.

@lavturo
Copy link
Contributor Author

lavturo commented Jan 15, 2020

@cmb69 I believe I made the current test file into a server test based on what you said. Is it correct?

@cmb69
Copy link
Member

cmb69 commented Jan 16, 2020

I believe I made the current test file into a server test based on what you said.

With "server test" I meant to actually run with some Webserver, not to include the file from the filesystem. However, that would probably be a step too much for now, and including 'HelloWorld.php" like you did is fine. After all, we now have the first working app test! :)

A minor issue: at first I used a PHP without php.ini, and PHPUnit-5.7.27\autoload.php wasn't generated (even though "Created autoload.php" was reported). I think we should catch this, and give a helpful error message (or at least don't go on with the tests). Could you have a look at that please?

@lavturo
Copy link
Contributor Author

lavturo commented Jan 16, 2020

My mistake. Hopefully we can work towards that.

As for the issue, you're correct. The autoload generator (phpab) says that it needs the following extensions enabled:

  • Fileinfo (ext/fileinfo)
  • Tokenizer (ext/tokenizer)
  • For PHAR generation support:
  • ext/phar (write enabled: phar.readonly = Off)
  • ext/gzip (optional)
  • ext/bzip2 (optional)
  • ext/openssl (optional, for phar signing only)

However, I tested with only the FileInfo extension enabled and it seems to work. So I updated the files to reflect that.

@cmb69
Copy link
Member

cmb69 commented Jan 17, 2020

Yes, fileinfo should be sufficient, since tokenizer is always statically compiled in on Windows, and we don't need to generate phars.

PR is fine now. Thanks! Applied as 280b673, bf0a98e, 246613f and b54b82f.

@cmb69 cmb69 closed this Jan 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants