Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Replace SimpleTests with PHPUnit #3227

Closed
sgiehl opened this Issue · 93 comments

5 participants

@sgiehl
Collaborator

This is a ticket related to #1470

We are going to migrate all tests to PHPUnit in order to be able to generate code coverage reports.

Note:

  • Before release, update tests/index output to explain the PHPUnit way
  • Update Jenkins to run the PHPUnit tests
  • Update tests/README.TXT
@mattab
Owner

I can't wait to see code coverage metrics :) Thank you again for tackling this difficult & long project of converting tests to a new framework!

@sgiehl
Collaborator

Already converted all core and plugin tests in the last few days to run with phpunit. But the integration tests are bit hard to migrate. Most of them are somehow still failing.

@sgiehl
Collaborator

(In [6489]) refs #3227 added first phpunit tests and phpunit xml configuration

@sgiehl
Collaborator

(In [6490]) refs #3227 added more phpunit tests

@sgiehl
Collaborator

(In [6491]) refs #3227 added test files

@sgiehl
Collaborator

(In [6493]) refs #3227 added/improved some more tests

@sgiehl
Collaborator

(In [6494]) refs #3227 added/improved some more tests

@sgiehl
Collaborator

(In [6495]) refs #3227 more and more tests completely converted :)

@sgiehl
Collaborator

(In [6496]) refs #3227 more tests

@sgiehl
Collaborator

(In [6497]) refs #3227 more tests

@sgiehl
Collaborator

(In [6498]) refs #3227 datatable renderer tests

@sgiehl
Collaborator

(In [6499]) refs #3227 more tests

@sgiehl
Collaborator

(In [6502]) refs #3227 added first database tests

@sgiehl
Collaborator

(In [6504]) refs #3227 more tests converted to PHPUnit

@sgiehl
Collaborator

(In [6505]) refs #3227 optimized/added phpunit tests

@sgiehl
Collaborator

(In [6507]) refs #3227 fixed some tests

@sgiehl
Collaborator

(In [6508]) refs #3227 fixed tests expecting an exception, as current phpunit version doesn't support expecting standard exceptions

@sgiehl
Collaborator

(In [6511]) refs #3227 added tests for uncovered methods

@sgiehl
Collaborator

(In [6512]) refs #3227 wrote some tests for Piwik_Access / added missing tests for Piwik_Unzip

@sgiehl
Collaborator

(In [6536]) refs #3227 added releasechecklist tests

@sgiehl
Collaborator

(In [6537]) refs #3227 prevent svn to replace the inline $Id's

@sgiehl
Collaborator

(In [6560]) refs #3227 added first versions of some integration tests. there are still many things to be improved. in order to make them running correct the HTTP_HOST config needs to be changed in phpunit.xml. some of the tests aren't running because of encoding problems. I'll maybe have a look at that later

@sgiehl
Collaborator

(In [6566]) refs #3227 added, improved integration tests

@sgiehl
Collaborator

(In [6568]) refs #3227 added more integration tests

@sgiehl
Collaborator

(In [6571]) refs #3227 added another integration test

@mattab
Owner

Thanks Stefan!! great changes & new features we will benefit with phpunit...

Looking forward to the end of the project, to enjoy PHPUnit fully and also avoid maintaining both test suites which will would be a challenge long term.

@sgiehl
Collaborator

(In [6574]) refs #3227 added integration test

@robocoder

(In [6582]) refs #3227 - allow devs to have a local, customized phpunit.xml

@mattab
Owner

Replying to SteveG:

(In [6560]) refs #3227 added first versions of some integration tests. there are still many things to be improved. in order to make them running correct the HTTP_HOST config needs to be changed in phpunit.xml. some of the tests aren't running because of encoding problems. I'll maybe have a look at that later

Would there be any way not to set the HOST in phpunit.xml and automatically read it from HTTP_HOST?

@sgiehl
Collaborator

As PHPUnit is executed in command line mode the HTTP_HOST variable is set to the hosts name. that might not be the same host apache is listening. It might also be possible you need to adjust REQUEST_URI as piwik might not be located in /piwik

@sgiehl
Collaborator

(In [6589]) refs #3227 improved / added more integration tests

@sgiehl
Collaborator

(In [6590]) refs #3227 more and more integration tests migrated :)

@mattab
Owner

(In [6592]) refs #3227 Adding bit of code in README + rename the phpunit.xml to the final name so developers just have to edit the file

@sgiehl
Collaborator

maybe we should leave the name of phpunit.xml.dist as it was, to allow devs to have a customized phpunit.xml without having done commitable changes.

@robocoder

Sebastian says the convention is phpunit.xml.dist would be the distributed config.

We would add phpunit.xml to.svn:ignore

@robocoder

Phpunit will try to load phpunit.xml before loading phpunit.xml.dist

@mattab
Owner

I just find that it isn't "dev friendly" to have to copy the file :)
Is there a way to avoid the file copy and simply have devs edit the host in a file?

@sgiehl
Collaborator

well, we could copy and adjust the phpunit.xml within the installation process, so no dev would have to edit anything...

@sgiehl
Collaborator

(In [6597]) refs #3227 and the last bunch of integration tests

@mattab
Owner

the last bunch of integration tests

Last ? Wow nice :-)

Note: I created related #3290 to use VisualPHPUnit to run tests in browser (which Benaka already setup)

@sgiehl
Collaborator

Yes. But I need to refactored some parts again. Many Integration tests aren't working by now and they are bit to slow.

Btw. I'll rename phpunit.xml to phpunit.xml.dist again as it is recommended and phpunit.xml.dist will be used if no phpunit.xml exists

@mattab
Owner

Replying to SteveG:

Btw. I'll rename phpunit.xml to phpunit.xml.dist again as it is recommended and phpunit.xml.dist will be used if no phpunit.xml exists

sorry I didn't know it would read the .dist, sounds good to rename it back!

@sgiehl
Collaborator

(In [6609]) refs #3227 renamed phpunit.xml again

@sgiehl
Collaborator

(In [6610]) refs #3227 added phpunit.xml to svn:ignore

@sgiehl
Collaborator

(In [6626]) refs #3227 refactored integration tests to setup database and tracking only once per class; that should speed up the tests a bit

@sgiehl
Collaborator

(In [6645]) refs #3227 integration tests: skip whole test suite if an error occurs
while setup (atm bulk tracking doesn't work)

@sgiehl
Collaborator

(In [6650]) refs #3227 fixed test

@sgiehl
Collaborator

(In [refs #3227, #3107, #3201 changed phpunit test according to changes in 6659)

@sgiehl
Collaborator

(In [6674]) refs #3227 fixed some tests; removed controller tests; updated readme

@sgiehl
Collaborator

(In [6675]) refs #3227 added missing plugin tests for LanguagesManager and PrivacyManager

@sgiehl
Collaborator

(In [6676]) refs #3227 removed class inheritance as that causes problems in some tests. improved and fixed some tests. now even more tests are running

@sgiehl
Collaborator

(In [6719]) refs #3227 fixed test

@sgiehl
Collaborator

(In [6720]) refs #3227 skip test, as it is skipped in simpletest aswell

@mattab
Owner

(In [Refs #3227 removing old proxy-piwik.php to use phpunit proxy/piwik.php instead (committed in 6743) )

@sgiehl
Collaborator

(In [6819]) refs #3227 improved some tests & added some still missing tests

@sgiehl
Collaborator

(In [6820]) refs #3227 finally found the difference - fixes some more integration tests

@sgiehl
Collaborator

(In [6868]) refs #3332, refs #3227 fixed possible fatal if no row is available for page metrics; do not run transition tests by default; ensure that all plugins are loaded before integration tests

@sgiehl
Collaborator

(In [6869]) refs #3227 ensure user language is set correct for reports

@diosmosis
Collaborator

(In [6870]) Refs #3227, get two tests to pass & speed up the PrivacyManager test a bit.

@sgiehl
Collaborator

(In [6871]) refs #3227 always reset language to en before each test

@diosmosis
Collaborator

(In [6882]) Refs #3227, speed up PrivacyManager some more.

@diosmosis
Collaborator

(In [6885]) Refs #3227, make sure API method metadata is reloaded when Piwik_API_Proxy::hideIgnoredFunctions is set to a new value.

@diosmosis
Collaborator

(In [Refs #3163, #3227, make sure no exception thrown in tracker when no 'ua' parameter & no HTTP_USER_AGENT. (fix for bug in 6737)).

@sgiehl
Collaborator

(In [6892]) refs #3227 unset pdf reports cache after each integration test

@sgiehl
Collaborator

(In [6894]) refs #3227 reset fakeaccess data to avoid problems between some tests

@sgiehl
Collaborator

(In [6897]) refs #3227 integration tests: check file length before trying to check if the string is equal for non xml output; that prevents some outofmemory exceptions for longer files
mark test as incomplete if an expected output file is missing

@JulienMoumne
Collaborator

(In [6938])

refs #3227

  • include back <ImageGraphUrl>

refs #2318

  • reset smarty cycle between each report
@diosmosis
Collaborator

(In [6980]) Refs #3330, #766, #3227 use RankingQuery and truncate tables as they are created in Actions plugin. Also modified phpunit integration testing mechanism so all API calls are tested and outputted before a test case throws.

@mattab
Owner

(In [7064]) Refs #3158 #3227

  • Fixing encoding bugs. We never noticed but there was a bug in the code (see change in XML files which are now correct).
  • Display Between X and Y even when values are the same (otherwise it looks like there's a bug)
  • fixing other tests I'm SO glad we have nice tests coverage for this complicated code otherwise we would be totally screwed!
@sgiehl
Collaborator

(In [7239]) refs #3227 starting to remove simple tests

@sgiehl
Collaborator

(In [7241]) refs #3227 removing more simple tests

@sgiehl
Collaborator

(In [7242]) refs #3227 removing integration simple tests

@sgiehl
Collaborator

(In [7244]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7245]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7246]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7247]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7248]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7249]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7250]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7251]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7252]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7253]) refs #3227 moving integration result files

@sgiehl
Collaborator

(In [7318]) refs #3227 fixing some tests

@mattab
Owner

Great it looks like it's very close to closing the ticket!

Code Feedback:

  • Can you confirm you have moved the expected files and so they haven't changed during migration?
  • I notice 4 remaining .test.php files to migrate/delete
  • I wonder about Plugins testability and PHPUnit running $pluginX tests files as a new test group (cf email)

Very nice! I cant wait now to enjoy Jenkins+PHPUnit....!

@sgiehl
Collaborator

(In [7456]) refs #3227 removing old benchmark tests

@sgiehl
Collaborator

(In [7487]) refs #3227 improvements to languagemanager tests; now every language is run as a seperate test case

@sgiehl
Collaborator

(In [7548]) refs #3227 migrated one remaining test

@sgiehl
Collaborator

(In [7549]) refs #3227 migrated last remaining test

@sgiehl
Collaborator

(In [7551]) refs #3227 removing remaining simpletest files

@sgiehl
Collaborator

(In [7553]) refs #3227 removed duplicate file

@sgiehl
Collaborator

Migration nearly finished. Last point on the list should now be: the testability of new plugins (having a official plugin directory in mind). But maybe we should create a seperate ticket for that or handle it as soon as it is realy required.

@mattab
Owner

Ok we will deal with the testability of Plugins in a new ticket. Could you create it, especially if you have some rought ideas on how it could be done with PHPUnit & ant?

Stefan, Kuddos for leading this project to completion! it's such a pleasure working with PHPUnit!

@mattab
Owner

Milestone 1.8.x Piwik 1.8.x deleted

@sgiehl sgiehl added this to the 1.12.x - Piwik 1.12.x milestone
@sgiehl sgiehl self-assigned this
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.