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

Test enhancement #62

Merged
merged 1 commit into from
Nov 17, 2018
Merged

Test enhancement #62

merged 1 commit into from
Nov 17, 2018

Conversation

peter279k
Copy link
Contributor

@peter279k peter279k commented Nov 16, 2018

Changed log

  • Set different PHPUnit versions to support multiple PHP versions.
  • Using class-based PHPUnit namespace to be compatible with latest PHPUnit version.
  • Drop the php-5.5 and php-5.6 versions support because these PHP versions will be end of life.
  • Add the namespace for the classes in tests folder.
  • Add the tests/bootstrap.php file to load the all required files once before doing unit test work.

composer.json Outdated
"require": {
"php": ">=5.5.0",
"ext-mbstring": "*"
},
"require-dev": {
"phpunit/phpunit": "4.*"
"phpunit/phpunit": "^4.8 || ^5.7 || ^6.5"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason for this? I think you should just bump it to >=6 ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adelowo, you're right. I just set the ^6.5 version to support the php-7.0+ versions :).

<?php

require_once __DIR__ . '/../vendor/autoload.php';
require_once __DIR__ . '/Fixtures/Even.php';
Copy link
Contributor

@adelowo adelowo Nov 16, 2018

Choose a reason for hiding this comment

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

Haven't written PHP in a long time but I remember that if you set up require-dev ( which you have), you don't have to manually require files except vendor/autoload.php.

Before tests are run, composer install --dev command is issued so it should take care of the autoloading.

Copy link
Contributor Author

@peter279k peter279k Nov 16, 2018

Choose a reason for hiding this comment

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

Yeah, I almost forgot that. I only need to do is loading the vendor/autoload.php and don't load that files you mention manually because of namespace definition.

@adelowo
Copy link
Contributor

adelowo commented Nov 16, 2018

LGTM... Using the namespaced version of PHPUnit is just awesome.

@peter279k
Copy link
Contributor Author

Hi @adelowo, thank you for your review :). I appreciate your help.

I've done for the request changes that you mention.

@adelowo
Copy link
Contributor

adelowo commented Nov 16, 2018

Hi @adelowo, thank you for your review :). I appreciate your help.

Sure thing @peter279k ...

You'd have to wait on @emsifa

@emsifa emsifa changed the base branch from master to v1 November 17, 2018 02:32
@emsifa emsifa merged commit 60373fd into rakit:v1 Nov 17, 2018
@emsifa
Copy link
Member

emsifa commented Nov 17, 2018

I merge this to v1 branch. Thank you.

@emsifa emsifa mentioned this pull request Nov 17, 2018
8 tasks
@peter279k peter279k deleted the test_enhancement branch November 17, 2018 02:43
@emsifa emsifa mentioned this pull request Nov 29, 2018
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

Successfully merging this pull request may close these issues.

None yet

3 participants