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

QA: Enable Widgets HTML compare integration tests #2908

Closed
robocoder opened this issue Feb 6, 2012 · 15 comments
Closed

QA: Enable Widgets HTML compare integration tests #2908

robocoder opened this issue Feb 6, 2012 · 15 comments
Assignees
Labels
Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Milestone

Comments

@robocoder
Copy link
Contributor

We would like to enable a new set of integration tests which call all known widgets, record output HTML in processed/ and then compare to expected/*.html files.

This will help us detect regressions in the Javascript/HTML rather than at the API level which we currently relying on.

This ticket will keep track of this work.

@mattab
Copy link
Member

mattab commented Feb 8, 2012

There are a few pending bugs with the widget compare mode.

  • There are dates in HTML outputs, so that changes every day
    piwik.startDateString = "2010-01-02";   piwik.endDateString = "2010-01-31"; piwik.minDateYear = 2010;   piwik.minDateMonth = parseInt("01", 10);    piwik.minDateDay = parseInt("02", 10);  piwik.maxDateYear = 2012;   piwik.maxDateMonth = parseInt("02", 10);    piwik.maxDateDay = parseInt("05", 10);  piwik.language = "en";</script><!--[if lt IE 9]>
  • The Piwik URL is found in the output
    piwik.piwik_url = "http://localhost/trunk/tests/";

It should be stripped out since jenkins/we all have different piwik urls.

  • The cache buster also changes
<link rel="stylesheet" type="text/css" href="themes/default/ieonly.css?cb=d269fbd1ca401f064e0d1a07049472c3" />
<span class="topBarElem"><a id="topmenu-widgetize" href="index.php?apiTestingLevel=none&amp;widgetTestingLevel=compare_output&amp;module=Widgetize&amp;action=index">Widgets</a></span> | 

I tried this patch but that wasn't enough to fix this issue so it is still pending:

Index: Integration.php
===================================================================
--- Integration.php (revision 5774)
+++ Integration.php (working copy)
@@ -92,6 +92,7 @@
    function setUp() 
    {
        parent::setUp();
+       $_GET = $_REQUEST = array();

        // Make sure translations are loaded to check messages in English 
        Piwik_Translate::getInstance()->loadEnglishTranslation();
@@ -119,6 +120,7 @@
    function tearDown() 
    {
        parent::tearDown();
+       $_GET = $_REQUEST = array();
        Piwik_Translate::getInstance()->unloadEnglishTranslation();

        // re-enable tag cloud shuffling
  • some tests are bound to fail, since they report a relative time since the data was created. For example, SEO widget contains:

<td><img style='vertical-align:middle;margin-right:6px;' src='plugins/SEO/images/whois.png' border='0' alt="Domain Age"> Domain Age
</td><td>
<div style='margin-left:15px'>
4&nbsp;years&nbsp;202&nbsp;days                                                 </div>  
</td>

which is a date in human readable form which will change every day. It should be stripped out before comparing html.

@mattab
Copy link
Member

mattab commented Feb 8, 2012

Anthon initial report:

  • When the integration tests run a second time (PDO_MYSQL then MYSQLI) on the CI server, an error is shown in the console log. Something missing from the setup/teardown?
[exec] <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
     [exec]    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
     [exec] <html>
     [exec] <head>
     [exec]     <title>Piwik &rsaquo; Error</title>
     [exec]     <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
     [exec]     <link rel="shortcut icon" href="plugins/CoreHome/templates/images/favicon.ico" />
     [exec]     <link rel="stylesheet" type="text/css" href="themes/default/simple_structure.css" />
     [exec] </head>
     [exec] <body>
     [exec] <div id="contentsimple">
     [exec]     <div id="title"><img title='Piwik' alt="Piwik" src='themes/default/images/logo-header.png' style='margin-left:10px' /><span id="subh1"> # <a href='http://piwik.org/'>web analytics</a></span></div>
     [exec] <p>The parameter 'idGoal' doesn't have a correct type, and a default value wasn't provided.</p>
     [exec]                 <p><a href="index.php">Go to Piwik</a><br/>
     [exec]                 <a href="index.php?module=Login">Login</a></p>
     [exec]                 <font color="#888888">Backtrace:<br /><pre>#0 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/plugins/Goals/Controller.php(57): Piwik_Common::getRequestVar('idGoal', NULL, 'string')
     [exec] #1 [internal function]: Piwik_Goals_Controller->widgetGoalReport()
     [exec] #2 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/core/FrontController.php(138): call_user_func_array(Array, Array)
     [exec] #3 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/core/FrontController.php(159): Piwik_FrontController->dispatch(NULL, NULL, NULL)
     [exec] #4 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/integration/Integration.php(765): Piwik_FrontController->fetchDispatch()
     [exec] #5 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/integration/Integration.php(1228): Test_Integration->callWidgetsCompareOutput('test_TwoVisitor...', 'all', Array, false, false)
     [exec] #6 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/integration/Integration.php(1112): Test_Integration_Facade->runControllerTests()
     [exec] #7 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/simpletest/invoker.php(68): Test_Integration_Facade->test_RunAllTests()
     [exec] #8 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/simpletest/invoker.php(126): SimpleInvoker->invoke('test_RunAllTest...')
     [exec] #9 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/simpletest/errors.php(49): SimpleInvokerDecorator->invoke('test_RunAllTest...')
     [exec] #10 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/simpletest/invoker.php(126): SimpleErrorTrappingInvoker->invoke('test_RunAllTest...')
     [exec] #11 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/simpletest/exceptions.php(43): SimpleInvokerDecorator->invoke('test_RunAllTest...')
     [exec] #12 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/simpletest/test_case.php(143): SimpleExceptionTrappingInvoker->invoke('test_RunAllTest...')
     [exec] #13 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/simpletest/test_case.php(595): SimpleTestCase->run(Object(HtmlTimerReporter))
     [exec] #14 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/simpletest/test_case.php(598): TestSuite->run(Object(HtmlTimerReporter))
     [exec] #15 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/TestRunner.php(111): TestSuite->run(Object(HtmlTimerReporter))
     [exec] #16 /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/tests/all_tests.php(19): TestRunner->run()
     [exec] #17 {main}</pre></font> <ul>
     [exec]                         <li><a target="_blank" href="?module=Proxy&action=redirect&url=http://piwik.org">Piwik.org homepage</a></li>
     [exec]                         <li><a target="_blank" href="?module=Proxy&action=redirect&url=http://piwik.org/faq/">Piwik Frequently Asked Questions</a></li>
     [exec]                         <li><a target="_blank" href="?module=Proxy&action=redirect&url=http://piwik.org/docs/">Piwik Documentation</a></li>
     [exec]                         <li><a target="_blank" href="?module=Proxy&action=redirect&url=http://forum.piwik.org/">Piwik Forums</a></li>
     [exec]                         <li><a target="_blank" href="?module=Proxy&action=redirect&url=http://demo.piwik.org">Piwik Online Demo</a></li>
     [exec]                         </ul></div>
     [exec] </body>
     [exec] </html>

@mattab
Copy link
Member

mattab commented Feb 8, 2012

The Widgets HTML integration tests were disabled in [5764]

@mattab
Copy link
Member

mattab commented Feb 8, 2012

(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
Copy link
Member

mattab commented Mar 9, 2012

(In [6014]) Refs #2908
remove .html widgets output until tests pass

@diosmosis
Copy link
Member

In de5af59: Refs #2908, refactored tests so database setup (adding sites, tracking visits) are separated from API tests. Put setup code into fixtures and reused code as much as possible.

@mattab
Copy link
Member

mattab commented Mar 11, 2013

Note: When the HTML changes, this should not "fail" the build but simply appear as a NOTICE message. Not sure how we can manage that, but it's important not to fail the build.

@halfdan
Copy link
Member

halfdan commented Mar 11, 2013

A much better way than comparing processed vs. expected from files is to check for expected data in the HTML. This is what's common practice for Ruby tests, e.g. https://github.com/piwik/piwik-ruby-tracking/blob/master/spec/views/piwik_tracking_tag.html.erb_spec.rb

These two tests just check for the URL and the correct idSite in the tracking code. This is much more flexible and prevents copy/paste to get a test passing.

@mattab
Copy link
Member

mattab commented Mar 11, 2013

I think we need 2 levels:

  • Widget HTML tests that test for exact HTML output. This can be done by developers by running phpunit --group Widgets.
    • This is not included in default build therefore do not fail the CI build.
    • this is useful for example, when we do a big refactoring, we can easily check that nothing has been broken. Similarly useful if eg. we upgrade the template engine to latest version.
  • a new test that will, given a test fixture, generate the HTML of all widgets, and:
    • will look for differences in the "text output" as you suggest
    • will also look for notices/warning/errors and other error cases e.g. Returned output < 300 characters
      When text is different or there is a notice/error, the build fails.

@diosmosis
Copy link
Member

@matt Regarding 'will look for differences in the "text output" as you suggest', what specifically will be looked for?

@mattab
Copy link
Member

mattab commented Mar 14, 2013

I'm not sure the best practises in the field, but I guess doing a "striptags" and looking at the "text" content (without html/js/css) could be a good basic test ?

@diosmosis
Copy link
Member

The issue is what to look for. Visits/actions correlate directly with individual reports, but not all reports together. So, if we look for data, we'd have to specify data for each individual report, which is like what is done now, but all in PHP instead of in 'expected' files. I'm not sure what else there is to look for.

@mattab
Copy link
Member

mattab commented Mar 17, 2013

I think we just strip_tags and look at all the content. This is like the expected file test, but it will greatly increase coverage and also test the Controller stack + Views/visualization + templates.

@halfdan
Copy link
Member

halfdan commented Mar 17, 2013

strip_tags is not a good idea. HTML can change quickly and data be represented differently. Using strip_tags will fail once you do a simple HTML reorder.

@mattab
Copy link
Member

mattab commented Aug 9, 2013

Testing for HTML is not necessarily a good idea. Instead we would like to generate screenshots and compare screenshots!

@robocoder robocoder added this to the 2.0 - Piwik 2.0 milestone Jul 8, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Projects
None yet
Development

No branches or pull requests

4 participants