Integration testing: list of ideas of improvement

[mattab]

This ticket is a placeholder for improvements related to integration tests and tests in general.

Check out existing tests (Main.test.php or Integration.test.php)

Improve tests and increase coverage:

• Widgets HTML output should be tested against an expected HTML output. See QA: Enable Widgets HTML compare integration tests #2908
• Add tests for the standard API filter parameters (filter_*)
• Add a test which fetches a given subtable (equivalent to clicking on a row to fetch the subrow)
• Add a test using API authentication with token_auth
• Some modules lack unit tests, we could add them:
• Goal API add/update/delete,
• LanguagesManager,
• SEO.getRank,
• Test that generated PDF, HTML & SMS reports, similarly to generated XML, don't change when not expected to. See Include scheduled reports in integration tests #3323

See also related ticket and comments for more ideas: #1470

[mattab]

(In [2877]) Refs #1465 Adding a file that loads and displays in the browser all widgets that are registered in the Piwik install.
This could ideally be loaded by Webtests to ensure that

• there is no JS error
• all widgets display correctly with data as expected

[mattab]

Random notes: Tracking algorithm could test that

• negative id site should not throw exception
• a new visitor starts the visit by clicking on an outlink on one of the alias host for the website. Is this considered a new visit, or should it be ignored?

[mattab]

Moving "Tracker should set cookies in the request and have a different IP at each request. Test that unique visitors counts is correct." to #1912

[mattab]

Are Javascript tests run in Hudson at the moment? I assume they are not, since Webtest doesn't have a proper browser?

[robocoder]

Correct, not running in Hudson. (This is mentioned in #1470.) I'll be looking into using Selenium.

[diosmosis]

I've got an idea on how to implement the integration tests for widgets/Controllers. The integration tests could be split up into smaller files similar to:

class OneVisitorTwoVisitsTest extends IntegrationTestBase
{
    public function getApiToTest()
    {
        return array(
            'Goals.this',
            'Actions.that' => array(
                'setDateLastN' => true,
                // other options
            )
        );
    }

    public function getControllersToTest()
    {
        return array('Goals.get', 'Actions.get');
    }

    protected function setUp()
    {
        parent::setUp();

        // add website & visits...
    }

    protected function doTest()
    {
        // do extra tests (testing stuff other than API/Controllers)
    }
}

IntegrationTestBase (or whatever it'll be called) will define a test_main() method that runs tests based on the results of getApiToTest() & getControllersToTest().

I think it could also be possible to simply add a method similar to the callGetApiCompareOutput method, but this would result in less readable code.

Sound like a good idea?

[mattab]

Refactoring Main.test.php in several files would be a good idea for sure.

But, maybe this is not required for this feature. For example, the same way we call callGetApiCompareOutput() we could callWidgetsCompareOutput() which would load a given set of widgets (or load ALL widgets) and compare the HTML output with the pre-recorded output.

You are welcome to do the refactoring but I wanted to point out the "Direct" way as well ;)

[diosmosis]

I thought of the 'direct' way too, but I thought I could kill two birds w/ one stone.

[diosmosis]

(In [5666]) Refs #1465. Added ability to test controller actions to integration tests. Added two controller tests, one that tests MultiSites.index & another that tests all testable controller methods.

Notes:

• Fixed PHP_Notices that were generated by the UserSettings plugin.
• Controller action testing behavior can be changed by setting the widget testing level. At the moment, the default level is set to CHECK_WIDGET_ERRORS which only checks for recoverable PHP errors. It can be set to skip widget testing or compare the widget output w/ expected files as well.
• Widget testing can be done at every access level ('anonymous', 'view', 'admin', 'superuser').
• Not all controller actions can be tested, those that can't (at the moment) are blacklisted from being tested when 'all' actions are tested.

[diosmosis]

(In [5667]) Refs #1465. Fixing the build (hopefully).

[diosmosis]

(In [5668]) Refs #1465. Refactored integration tests by splitting each individual test in Main.test.php up into separate files.

NOTES:

• New tests use the Test_Integration_Facade type that makes the tests easier to read.
• Split Database.test.php into two types (in the same file) so the hello world test cases aren't duplicated every time another type derives from it.
• Number of passes changed from 10233 to 10278 for all tests. The increase is due to further splitting up of certain tests.

[mattab]

capedfuzz, thanks for the improvements and contributions!

Code review:

** Controller action testing behavior can be changed by setting the widget testing level. At the moment, the default level is set to CHECK_WIDGET_ERRORS which only checks for recoverable PHP errors. It can be set to skip widget testing or compare the widget output w/ expected files as well.

• I think this is a good start, but can we enable COMPARE_WIDGET_OUTPUT mode. It would be very useful to spot when widgets output change and we do not expect it.
  • The .html outputs of all widgets tested would be stored in a directory the same way the API outputs, for example in tests/integration/expected-widgets/Referers.getKeywords.html
  • The controller "test all methods" would be called for only 1 test maybe, since it will be slow to run, we need only run it on one test case
• tearDown()
  // add access to all test users if doing controller tests
  Why do you add access in the teardown? tear down should delete all created users and access, and setUp() would create users + access
• Great work on the Facade refactoring. It makes the test a lot more readable and easier to manage.
  • Can you please confirm that you have carefully copy paste all tests and haven't had to do any modification to the code that could/should have impacted the tests?
• How to replicate the old Main.test.php behavior, and execute all integration tests (now split in different files) automatically?

[diosmosis]

Replying to matt:

• I think this is a good start, but can we enable COMPARE_WIDGET_OUTPUT mode. It would be very useful to spot when widgets output change and we do not expect it.
  • The .html outputs of all widgets tested would be stored in a directory the same way the API outputs, for example in tests/integration/expected-widgets/Referers.getKeywords.html
  • The controller "test all methods" would be called for only 1 test maybe, since it will be slow to run, we need only run it on one test case

Right now I added a test that tested all methods, but doesn't compare the widget output (its in the OneVisitorTwoVisits test). This could be changed, but that would mean looking through all the controller output, which I didn't have time for. If you think it'd be ok to assume what's generated right now is correct, I can make the change right now.

• tearDown()
  // add access to all test users if doing controller tests
  Why do you add access in the teardown? tear down should delete all created users and access, and setUp() would create users + access

That code isn't in tearDown, it's in the createWebsite function.

• Great work on the Facade refactoring. It makes the test a lot more readable and easier to manage.
  • Can you please confirm that you have carefully copy paste all tests and haven't had to do any modification to the code that could/should have impacted the tests?

It's been a bit since I made the changes, so I'll have to look it over again, but I remember only making a few changes to the CsvExport tests (regarding changing the language). Also, the tests all passed w/o modification and I was able to explain the increase in passes. I'll look through the commit to double check, though.

• How to replicate the old Main.test.php behavior, and execute all integration tests (now split in different files) automatically?

I guess I could add a test group for this? Or maybe just a file like all_tests.php that only looks for integration tests? A little while ago I wrote a small alternative to all_tests.php that ran each test via AJAX so you would be able to run tests individually and see each test's progress after it finished. I was going to email it in case it was useful, but before I do, I could add a way to run all integration tests.

[mattab]

Right now I added a test that tested all methods, but doesn't compare the widget output (its in the OneVisitorTwoVisits test). This could be changed, but that would mean looking through all the controller output, which I didn't have time for. If you think it'd be ok to assume what's generated right now is correct, I can make the change right now.

Please enable this feature for now (I will disable later if need be).

You can copy all the widgets output as long as they seem to work and return data.

I guess I could add a test group for this? Or maybe just a file like all_tests.php that only looks for integration tests?

Perfect idea to add a script to trigger only integration tests.

However, if you also enable HTML Widget comparisons, would it make sense to have run_integration_tests script to run either Widget HTML compare, API html compare, or both (which would be the default mode)?

Thanks!!

[mattab]

(In [5739]) * Refs #534 Removing exit_rate from metadata API, it was added in the standard processed metrics causing other reports to display it with 0% values

• Refs Integration testing: list of ideas of improvement #1465 Leave underscores in test names just to keep tests more readable (even though it is not a best practise to have _ in class prefixes!!)

[mattab]

(In [5743]) Refs #1465
Kuddos cappedfuzz for the great refactoring!

Fixes bug that a subset of the previous tests were ignored.
FYI: To find out these missing tests, I deleted all files in processed/ directory, then ran all_tests.php - I then noticed maybe 40 files or so, that were in expected/ but not in processed/ meaning they were not generated but they should have been.

[diosmosis]

(In [5747]) Refs #1465. Added all_integration_tests.php script that invokes all integration tests and nothing else, and refactored all_tests.php. Also added the ability to disable API integration tests & modify the widget testing level via the browser.

NOTES:

• Removed widget tests for ExampleRssWidget since it depends on a feedburner URL.

[diosmosis]

Just committed a change that allows you to execute all integration tests, but nothing else. Also, its possible to disable API testing & modify the widget testing level by setting the apiTestingLevel & widgetTestingLevel query params. So,

/tests/all_integration_tests.php?apiTestingLevel=none&widgetTestingLevel=compare_output

will skip the API tests & compare widget output. These are the possible values for both params:

• apiTestingLevel: 'none' or 'compare_output'
• widgetTestingLevel: 'none' or 'compare_output' or 'check_errors'

[mattab]

(In [5752]) Refs #1465

• Added tests/indexs.php as a welcoming page for piwik devs, to link to the most commonly used tests
• Linked to the new capabilities of testing Widget for errors and/or compare HTML
• slightly refactored/renamed

[diosmosis]

(In [5764]) Refs #1465, disabled widget comparison in order to get jenkins build to pass. Can't reproduce the errors jenkins gets.

[robocoder]

After r5764, I "downgraded" the CI server from PHP5.4.0RC5 to PHP5.3.10 since that's a security release and needs to be tested. Not sure if that's coincidental with the build currently failing though.

[robocoder]

(In [5768]) refs #1465 - comment out output buffering; appears to be a side effect of r5752

[Sun Feb 05 10:44:17 2012] [warn] [client 127.0.0.1] Timeout waiting for output from CGI script /home/www/bin/php-cgi, referer: http://localhost/piwik/tests/all_integration_tests.php
[Sun Feb 05 10:44:17 2012] [error] [client 127.0.0.1] (70007)The timeout specified has expired: ap_content_length_filter: apr_bucket_read() failed, referer: http://localhost/piwik/tests/all_integration_tests.php

[robocoder]

The CI build is currently "passing" with php5.3.10 (after reverting r5703) but this might be a false positive because I'm seeing what looks like an error in the build's console log. Will need to diagnose that before testing on php5.4.0rc7.

[robocoder]

(In [5770]) refs #1465 - revert r5768; add paintMethodEnd() to flush buffers intermittently

[mattab]

Widget HTML compare feature moved to its own ticket: #2908

[mattab]

(In [5783]) Refs #1465 Trying to remove the message reported by Anthon in http://qa.piwik.org:8080/jenkins/job/Piwik/2837/consoleFull

[mattab]

(In [5785]) Refs #1465 Completely disabling widget testing since it fails for mysqli for some unknown reasons (to be investigated later)
Refs #2908

eg. See log in http://qa.piwik.org:8080/jenkins/job/Piwik/2838/consoleFull

[mattab]

@vipsoft Tests now pass on mysqli without the message after disabling widget output compare

[julienmoumne]

adding reference to #3323

[mattab]

all these items are now done IMHO

The only remaining idea is to enable integration test for widgets covered in #2908