Skip to content

New Feature/HTTP Testing capabilities #1047

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

Merged
merged 11 commits into from
May 26, 2018
Merged

New Feature/HTTP Testing capabilities #1047

merged 11 commits into from
May 26, 2018

Conversation

lonnieezell
Copy link
Member

Includes refactoring of a few core portions of the framework (like CodeIgniter.php and IncomingRequest) to allow more flexible testing.

Docs still need to be written, but since @jim-parry is working on a lot of tests I want to get these changes in place so he can work with them.

The biggest things to know when working on tests:

  • any use of setUp() methods within tests must now call parent::setUp().
  • Any classes within tests/_support must have their namespace changed to Tests\Support\...

Biggest new feature is the FeatureTesting portion.

@lonnieezell
Copy link
Member Author

Ugh. Have an issue with Code Coverage and the debug toolbar.... will have to get that working later.

@jim-parry
Copy link
Contributor

Oh my - a couple of changes :)
I see that the unit tests break :(
Once that is resolved, go ahead and merge this. I will absorb and accommodate.

@lonnieezell
Copy link
Member Author

The tests themselves are passing but when it runs the code coverage it has issues with the toolbar loader. Seems like we had this issue in the past but I don’t remember how to fix it. Will try to get it working tonight.

@lonnieezell lonnieezell merged commit 34074ac into develop May 26, 2018
@lonnieezell
Copy link
Member Author

@jim-parry merged. Let me know if you have any questions.

@jim-parry
Copy link
Contributor

Not sure if I am doing something wrong, or if there is some config bit that differs between us, but I barely get a peep out of phpunit...

bash-4.1$ phpunit
PHP Fatal error:  Class 'CodeIgniter\Test\CIUnitTestCase' not found in /pub7/htdocs/CodeIgniter4/tests/_support/CIUnitTestCase.php on line 5

@lonnieezell
Copy link
Member Author

Since you got some other commits on your PR does that mean you got it working?

@jim-parry
Copy link
Contributor

No. A straight pull of "develop" leads to the fatal error above when I try to run phpunit locally.
Digging further to see what is different between the repo and my local project ... has to be some git ignored config thing IMHO.

@jim-parry
Copy link
Contributor

Hmm - I have the same problem if I do a straight download of the repo & try to run phpunit.
Running PHP 7.1.4 & phpunit 7.1.5, not sure if that matters. You?

In tests/_support/CIUnitTestCase, starting with "use CodeIgniter\Test; class CIUnitTestCase extends Test\CIUnitTestCase ..." gives the class not found error, while starting with "use CodeIgniter\Test\CIUnitTestCase; class CIUnitTestCase extends CIUnitTestCase ... " gives "Cannot declare class CIUnitTestCase because the name is already in use..."

At the moment, I suspect a namespace issue with CIUnitTestCase; not sure if it is somehow version related.

@jim-parry
Copy link
Contributor

jim-parry commented May 26, 2018

@lonnieezell I am leaning towards a breaking change in tests/_support/_bootstrap.php ... it no longer adds namespaces or loads the CI config or framework instance.

@lonnieezell
Copy link
Member Author

The changes in bootstrap are intentional. It was refactored so that a new instance of CodeIgniter was built for every test run, allowing us to do tests that were impossible previously.

I was running into similar things previously. That should be changed to a require_once I believe, though not sure exactly where it’s trying to do that. Confused how it worked for both me and for Travis though.

@jim-parry
Copy link
Contributor

The only thing "required" in _bootstrap is CIUnitTestCase. There is no mention of CodeIgniter or App config. Is this the way it is supposed to be? It feels incomplete.

@lonnieezell
Copy link
Member Author

It only feels incomplete because that functionality was moved.

I won't have time to look at it until maybe tonight.

@deathart deathart mentioned this pull request May 26, 2018
@lonnieezell
Copy link
Member Author

@jim-parry I'm not sure what to tell you. I just pulled down a new copy into a vagrant instance I have running with Ubuntu and tests run just fine. I can't replicate the issue you're describing here.

@jim-parry
Copy link
Contributor

I have setup centos6, centos7 & ubuntu instances, with php7.0, 7.1 & 7.2 (as appropriate), and phpunit 6 and 7. I have not found a combination that does not give the same problem.
I tried my laptop too, and same thing.
i have unit tests in other CI3-based projects, and they test fine.
I have found lots of references to similar problems online, but no good explanation or helpful solutions.
My guess is that I am doing something wrong, and I continute to dig :(
Thanks for checking into it.

@lonnieezell
Copy link
Member Author

That's crazy. I'm sorry it's doing that to you. My vagrant box is PHP 7.1.3, phpUnit 7.1.5, under Ubuntu 16.04.2 if that helps anything. Looks like on that box XDebug is not installed - which would not have ran code coverage, so is probably skipping over whatever is causing that issue. Dang.

On my mac I'm running PHP 7.1.14, with phpUnit 7.1.5 with XDebug enabled.

Have you tried just running any single test file to see if there's one particular file that's causing the issue? I know I had some painful times locating some of the things last week where I had to go one directory at a time running the tests. Once I even had to modify phpunit.xml's testsuite to start specifically including directories to see if there was some combination of folders causing errors I was seeing. It wasn't overly fun but eventually got me there. It sounds like the autoloader isn't being loaded somewhere. So I'd start stepping through testsuite's bootstrap to see where things died, too.

@jim-parry
Copy link
Contributor

Echoing the different paths shows BASEPATH as .../system instead of .../CodeIgniter4/system!
I see no reason that should be.
Modifying application/Config/Paths line 22 to public $systemDirectory = 'system'; lets my build proceed.
The mystery deepens!

@lonnieezell
Copy link
Member Author

@jim-parry Where is your phpunit command running from? Mine is running from the vendor/bin/phpunit folder. Do you have a symlink or phar loaded in the main file? If so, I wonder if that's messing with the URLs...

@jim-parry
Copy link
Contributor

phpunit 7.1.5
If I run it as "phpunit", it blows up with the CIUnitTestCase not found
If I run it as "vendor/bin/phpunit" (yes, second copy), it runs the unit tests and blows up generating clover xml code coverage report (mkdir file exists).
"phpunit" is a phar, while "vendor/bin/phpunit" is composer installed

@jim-parry
Copy link
Contributor

vendor/bin/phpunit is a symlink, to vendor/phpunit/phpunit/phpunit (a 1.2KB script)

@jim-parry
Copy link
Contributor

Problem resolved, at least on my system.
The change to line 22 of Config/Paths was the charm.
Why are the directory settings in that relative to the "public" folder? That sounds like it will get messy for someone who builds an app on CI4 which has been composer installed or included as a git submodule. Is there anything wrong with making them relative to the project root folder, i.e. ROOTPATH?
ps - I have had to increase the memory limit when running unit tests, to accommodate the code coverage.

@lonnieezell
Copy link
Member Author

Does the site still run from the frontend? Those paths have always been set relative to the public folder since that's where the index.php file is that runs everything. If you managed to make it work for both testing and front-end, then cool. Otherwise, it's just the way CodeIgniter has always been...

@jim-parry jim-parry deleted the testing branch September 27, 2018 03:40
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.

2 participants