Skip to content

Set CIDatabaseTestCase and CIUnitTestCase abstract#3789

Merged
samsonasik merged 2 commits intocodeigniter4:developfrom
willnode:develop
Oct 21, 2020
Merged

Set CIDatabaseTestCase and CIUnitTestCase abstract#3789
samsonasik merged 2 commits intocodeigniter4:developfrom
willnode:develop

Conversation

@willnode
Copy link
Copy Markdown
Contributor

Description
This is PR I propose to fix #3788 (my own issue)

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@samsonasik
Copy link
Copy Markdown
Member

Looking at your config at #3788, I think your PHPUnit config can use addUncoveredFilesFromWhitelist="true" instead:

		<whitelist addUncoveredFilesFromWhitelist="true" processUncoveredFilesFromWhitelist="true">

                </whitelist>

@willnode
Copy link
Copy Markdown
Contributor Author

I did try that, but I get error.

PHP Fatal error:  Cannot declare class App\Commands\FetchTemplate, because the name is already in use in /home/travis/build/domcloud/dom-portal/app/Commands/FetchTemplate.php on line 11

Because it's the first file in my app folder, I assumed somehow phpunit redefine App namespace again, but I have no idea what's causes nor how to solve it. Except turning off these settings.

Full build log or the project if that's necessary.

@samsonasik
Copy link
Copy Markdown
Member

The redeclare error seems already fixed in latest dev-develop at #3772 . Please try latest dev-develop with use builds script at like at https://github.com/codeigniter4/CodeIgniter4/blob/develop/admin/starter/builds and run:

php builds development && composer update

@willnode
Copy link
Copy Markdown
Contributor Author

Thanks. I tried, but that didn't work for me. I don't know why. I think I will revisit that problem later. I'm happy with my setup.

By the way, I seen that a lot phpdoc references changed to Interface due to this commit? That breaks intelephense in VS code (all references goes red, because they don't support @mixin yet?). I'm reverting back to release builds.

@samsonasik
Copy link
Copy Markdown
Member

Docblock check is part checked by PHPStan. The PHPStan configuration is at https://github.com/codeigniter4/CodeIgniter4/blob/develop/phpstan.neon.dist .Please create issue if you think there issue with that.

@willnode
Copy link
Copy Markdown
Contributor Author

Will do later, thanks.

Anyway, any chance this get merged? There maybe some alternatives I don't aware, of course.

@samsonasik
Copy link
Copy Markdown
Member

Lets wait other review from the team for it.

@samsonasik
Copy link
Copy Markdown
Member

I tried your repo, and after run: php builds development && composer update, and update phpunit.xml.dist:

		<whitelist addUncoveredFilesFromWhitelist="true" processUncoveredFilesFromWhitelist="true">
			<directory suffix=".php">./app</directory>
			<exclude>
				<directory suffix=".php">./app/Views</directory>
				<directory suffix=".php">./app/Config</directory>
				<file>./app/Config/Routes.php</file>
			</exclude>
		</whitelist>

running tests is succeed:

➜  dom-portal git:(master) ✗ vendor/bin/phpunit tests                    
PHPUnit 8.5.8 by Sebastian Bergmann and contributors.

Error:         No code coverage driver is available

....                                                                4 / 4 (100%)

Time: 1.97 seconds, Memory: 10.00 MB

So It probably related with your local dev system.

Copy link
Copy Markdown
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Current conversation aside, I think this is actually a fine change on its own.

Copy link
Copy Markdown
Member

@paulbalandan paulbalandan left a comment

Choose a reason for hiding this comment

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

Making the classes as abstract is not bad since these are not meant to be tested individually.

Project side, I think there is something wrong in your setup causing these to be tested. Here's a portion of my phpunit config.

    <testsuites>
        <testsuite name="default">
            <directory suffix="Test.php">tests/src</directory>
        </testsuite>
    </testsuites>

     <filter>
        <whitelist processUncoveredFilesFromWhitelist="true">
            <directory suffix=".php">src</directory>
        </whitelist>
    </filter>

@samsonasik samsonasik merged commit 2e904a4 into codeigniter4:develop Oct 21, 2020
@samsonasik
Copy link
Copy Markdown
Member

@willnode merged, thank you.
@MGatner @michalsn @paulbalandan thank you for the review.

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.

Bug: Warning no tests found in CodeIgiter\Tests\...

5 participants