joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.77k stars 3.65k forks source link

Travis shows build error without additional/necessary information #14060

Closed Napoleon-BlownApart closed 7 years ago

Napoleon-BlownApart commented 7 years ago

I'm having difficulties identifying what exactly is wrong with the changes I've proposed in Issue: Inheritable menu item property (take 2) #13817. The build passes for Job #33632.1 (PHP5.6 with parameters RUN_PHPCS="yes" RUN_UNIT_TESTS="no" INSTALL_MEMCACHE="no" INSTALL_MEMCACHED="no" INSTALL_REDIS="no") but fails for all the other configurations (except the JavaScript tests). These links are based on the commits made at the end of the current thread.

Looking at the Travis' output for Build #33632.5, the other PHP5.6 configuration, the error is stated in lines 1008 and 1110 as:

.....S.....S.......................FPHP Fatal error:  Call to a member function getActive() on null in /home/travis/build/joomla/joomla-cms/libraries/cms/application/site.php on line 323

Fatal error: Call to a member function getActive() on null in /home/travis/build/joomla/joomla-cms/libraries/cms/application/site.php on line 323

However I cannot replicate this error using my local repo, and others who've tested my proposal have not had the problem either. How can I fix a problem that I can't replicate?

In my attempts to figure this out (specific to my proposal), upon expanding line 189, I noticed that the inheritable field in jos_menu is not defined in line 522, where it should be (immediately after access). I don't understand how this could happen because the field is defined in all the the ./installation/sql/.../joomla.sql scripts for mysql, postgresql, and sqlazure, as well as in the update scripts in ./administrator/components/com_admin/sql/updates/.../3.7.0-2017-01-27.sql for mysql, postgresql, and sqlazure.

Why isn't Travis indicating which test failed? And, why is Travis not reporting the call stack that lead to the error?

photodude commented 7 years ago

it's not telling you what test failed, since it's not a test failure but a code failure. Something is causing null to be passed to getActive()

you could try debugging travis by telling phpcs to --debug so you get each test printed out... it might tell you which test ran when the code issue happened or the test before the current running test when the code issue happened.

Napoleon-BlownApart commented 7 years ago

@photodude: cheers. The Travis build is happening automatically after each push of my repo. How would I set the --debug switch? Alternatively, is there a way I can set-up a Travis test environment that mimics what is used here?

yvesh commented 7 years ago

@Napoleon-BlownApart just run the php unit tests locally.. e.g. do composer install at joomla root and run libraries/vendor/bin/phpunit . --debug --testdox (or only the suite that is failing, which is faster)

Napoleon-BlownApart commented 7 years ago

ok, I'll try that.

photodude commented 7 years ago

@Napoleon-BlownApart you just need to create your own account on travis and link it to your github then activate your fork of Joomla to do your own testing in the travis environment. Once you have that link all set up this url will exist https://travis-ci.org/Napoleon-BlownApart/joomla-cms

you make the --debug in the .travis.yml file , just remember to not push branches as PRs with --debug or other debugging items in the branch.

Napoleon-BlownApart commented 7 years ago

@yvesh: thnx. I installed composer locally and executed the command ./composer.phar libraries/vendor/bin/phpunit . --debug --testdox however I don't have a file named phpunit in libraries/vendor/bin, nor anywhere else in the Joomla tree, and the command returned the following error:

  [Symfony\Component\Console\Exception\CommandNotFoundException]
  Command "libraries/vendor/bin/phpunit" is not defined.

I'll check that I'm executing composer correctly.

photodude commented 7 years ago

@Napoleon-BlownApart you need to do composer as a --require-dev otherwise phpunit will not install as it's a dev only tool

Napoleon-BlownApart commented 7 years ago

@photodude: I setup my Travis acct, linked it to github and activated my Napoleon-BlownApart/joomla-cms fork. Nothing significant has happened.

I have my upstream set to joomla/joomla-cms, and each time I do a push, it goes into the PR. So I'm not quite sure where/how I should be setting the --debug flag. I'lll try putting it into my local .travis.yml and see. I will remove the flag once I've finished debugging.

Your comments are most welcome.

Napoleon-BlownApart commented 7 years ago

@photodude : I installed composer from https://getcomposer.org/download/ . Where do I put the --require-dev?

I tried ./composer.phar --require-dev libraries/vendor/bin/phpunit . --debug --testdox but it returned the same error. So I presume it's an installation issue?

Napoleon-BlownApart commented 7 years ago

@photodude : where do I put the --debug in the .travis.yml file? in the before_script or script sections?

photodude commented 7 years ago

Now that you have a travis account set up, you have to push something to your fork for travis to start a build.

as for the local, you are combining two different things, composer and phpunit, you need to do each one separately

as for --debug in the .travis.yml file.... it's a switch you add to phpunit. See the PHPunit documentation for details on how that works.

yvesh commented 7 years ago

Skip composer.phar it's just. Composer is only for installing dependencies and by default dev dependencies are installed too

./libraries/vendor/bin/phpunit . --debug --testdox

photodude commented 7 years ago

@Napoleon-BlownApart FYI php 7 often tells more of the story... for example in your case here are the tests failing (all from the previously known Call to a member function getActive() on null )

1) JApplicationSiteTest::testGetParams
Error: Call to a member function getActive() on null
/home/travis/build/Napoleon-BlownApart/joomla-cms/libraries/cms/application/site.php:323
/home/travis/build/Napoleon-BlownApart/joomla-cms/tests/unit/suites/libraries/cms/application/JApplicationSiteTest.php:202
2) JApplicationSiteTest::testGetPathway
Error: Call to a member function getActive() on null
/home/travis/build/Napoleon-BlownApart/joomla-cms/libraries/cms/pathway/site.php:34
/home/travis/build/Napoleon-BlownApart/joomla-cms/libraries/cms/pathway/pathway.php:93
/home/travis/build/Napoleon-BlownApart/joomla-cms/libraries/cms/application/cms.php:518
/home/travis/build/Napoleon-BlownApart/joomla-cms/libraries/cms/application/site.php:385
/home/travis/build/Napoleon-BlownApart/joomla-cms/tests/unit/suites/libraries/cms/application/JApplicationSiteTest.php:220
3) JApplicationSiteTest::testGetTemplate
Error: Call to a member function getActive() on null
/home/travis/build/Napoleon-BlownApart/joomla-cms/libraries/cms/application/site.php:434
/home/travis/build/Napoleon-BlownApart/joomla-cms/tests/unit/suites/libraries/cms/application/JApplicationSiteTest.php:244
4) JApplicationSiteTest::testRender
Error: Call to a member function getActive() on null
/home/travis/build/Napoleon-BlownApart/joomla-cms/libraries/cms/application/site.php:434
/home/travis/build/Napoleon-BlownApart/joomla-cms/libraries/cms/application/site.php:748
/home/travis/build/Napoleon-BlownApart/joomla-cms/tests/unit/core/reflection/reflection.php:79
/home/travis/build/Napoleon-BlownApart/joomla-cms/tests/unit/suites/libraries/cms/application/JApplicationSiteTest.php:306
Napoleon-BlownApart commented 7 years ago

@photodude : thanks! I have php7.1 (cli) and 7.0.15-1+deb (cgi) on my dev server. Using my Travis account, I'm going to create a new fork, make the changes, fix what needs fixing, and then create the pull request. It will make it a lot easier to evaluate what I'm proposing than continuing with the mess I have at the moment. I'm sure the above info will be useful. Cheers.

Napoleon-BlownApart commented 7 years ago

@photodude: Yes, on my Travis instance I can see the errors in the 7.0 log. Thanks, and I'll get onto fixing things up.

A question up front: What if there is a need to change a test? (Hopefully I don't.)

Napoleon-BlownApart commented 7 years ago

@yvesh : I have installed composer and phpunit, but not in the joomla-cms folder. The path to my phpunit is ~/.composer/vendor/bin and it's a symlink to ~/.composer/vendor/phpunit/phpunit/phpunit (which is of course, the executable). I have my $PATH setup to find it in ~/.composer/vendor/bin.

I have also installed phpunit/php-invoker and phpunit/dbunit using composer global require. They are all in the same location as describe above.

Is this a problem? Will phpunit find ./libraries/vendor in my joomla-cms tree?

yvesh commented 7 years ago

It's not an issue to install phpunit globally.. as long as you use the right version, we are still at the 4 .8 one..

You can run phpunit then by navigating into the joomla cms root directory (cloned from github) and execute

phpunit .

That's it. Ping me if you need any further help! If you want we can also invite you to the Joomla public Testing & CI Group on Glip (Chat client)

screenshot 2017-02-17 10 03 11

Napoleon-BlownApart commented 7 years ago

@yvesh : Ok, my phpunit is 6.0.6. Looks like I'll have to change that. An invite to the chat could be very useful, thanks. I haven't used Glip before though.
Also, if you could help me find some documentation about the tests, I would be grateful.

yvesh commented 7 years ago

@Napoleon-BlownApart send me your mail address to yves.hoppe [at] community.joomla.org

Napoleon-BlownApart commented 7 years ago

Going through the build output, the 1st step was to clone the 'inheritable' branch. I did that on my machine with the same clone string, and checked the ./installation/sql/mysql/joomla.sql file. On line 1361, the inheritable field is defined for the #__menu table. Yet, in line 523, further down in the output, instead of specifying the inheritable field, the build goes directly to the next field, which is img.

What happened to line 1361 of the SQL file? (Where is the SQL schema Travis is executing coming from?)
I'll answer my own question: There is a set of table definitions used specifically for testing in ./tests/unit/schema/.

Why is it necessary to have this separate definition when a full install and upgrade SQL has been defined elsewhere?

mbabker commented 7 years ago

The test suite purposefully uses a schema that is more isolated than the production schema and also includes things that won't exist in production. The entire production schema is not needed for unit testing coverage.

Napoleon-BlownApart commented 7 years ago

With the TEST sql updated, all the tests have passed.

Why is it necessary to have this separate SQL definition when the SQL for a full install and upgrade have already been defined elsewhere? And to use this for testing purposes as well?

Napoleon-BlownApart commented 7 years ago

Then you're not testing what has been coded.

Napoleon-BlownApart commented 7 years ago

@mbabker : you've been helping me from the start, and I really appreciate it. I could not have done this otherwise.

But to think that I spent a almost 2 weeks on this 'got ya' that doesn't make sense to me leaves me bewildered.

mbabker commented 7 years ago

Unit tests shouldn't be executing real database queries anyway. That's what functional testing is for. And functional testing on the actual database drivers should involve a very carefully crafted schema explicitly for testing the database API (which is the primary point of having the test schema to begin with).

Can this be improved? Probably. A lot of the test architecture isn't in an optimal state right now. Either way I wouldn't suggest that unit or functional tests default to the production schema.

Napoleon-BlownApart commented 7 years ago

I understand what you're saying, however, rather than implementing yet another schema (thus increasing the chance of making a mistake), why not use the schema defined in the main body of the application. A symlink?

I'm really happy that I've worked it all out. I really appreciate the assistance I've received from a bunch of people I've never dealt with in the past. Thank you all.

Napoleon-BlownApart commented 7 years ago

lmao..... now what? Drone doesn't like my comments above so it's flagged me as a fail.

photodude commented 7 years ago

Open the drone test and look at what code style issues you need to fix.

Napoleon-BlownApart commented 7 years ago

@photodude : yes, I had to refresh my page to see the full output. Done, fixed, and pushed. Cheers.

Napoleon-BlownApart commented 7 years ago

Before I close this issue, here are the steps I used to install phpunit and dbunit outside of the joomla-cms repository. Doing it this way enabled me to keep the repository clean of changes that result from the installation of phpunit.

I have, in my .bashrc the following exports:

export PATH=$PATH:~/.composer/vendor/bin
export COMPOSER=~/.composer/composer.json
export COMPOSER_VENDOR_DIR=~/.composer/vendor

Composer recognises another environment variable COMPOSER_HOME, that may achieve the same result without needing two exports. I didn't try this as it was at the bottom of the help page and I had mine working before I noticed it.

Using Composer, I installed:

composer require phpunit/phpunit=4.8.31
composer require phpunit/dbunit=1.3

Thanks again for all the assistance! :)

yvesh commented 7 years ago

@Napoleon-BlownApart you missed the word global ;) And you can use ~ to install the latest version of a package of that version.

composer global require phpunit/phpunit=~4.8
composer global require phpunit/dbunit=~1.3

Btw. i never needed the composer exports.