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

Integration Tests refactor (move API testing logic to separate classes) #5884

Merged
merged 11 commits into from Aug 1, 2014

Conversation

diosmosis
Copy link
Member

Refactoring of IntegrationTestCase for code clarity and cleanliness. Reduces the size of IntegrationTestCase.php and removes a lot of testing hacks.

Includes the following changes:

  • moved all API URL generation logic & API response handling logic to separate classes
  • created ApiTestConfig class to hold API test options (and documentation for properties)
  • remove unnecessary IntegrationTestCase properties and methods (including apiNotToCall/apiToCall)
  • remove as many unnecessary API test options as possible (including abandonedCarts, hackDeleteRangeArchivesBefore)
  • rename checkRequestResponse to assertApiResponseHasNoError
  • cleaned up API/Request::__construct handling of arrays (allows arrays to be used as test requests instead of actual URLs)

Verification:

I verified that all tests are being executed (master & this branch execute the same test cases and same API test URLs). There is a difference in the number of assertions reported by phpunit, however:

Integration 998 (master) => 997 (tests refactor)
Plugin 2489 (master) => 2481 (tests refactor)
Core 1257 (master) => 1259 (tests refactor)

I can't locate the cause of the discrepancy, but I don't think it matters considering the same amount of API urls and test cases are tested.

diosmosis added 4 commits July 26, 2014 15:04
…eleted by database drop in fixture setup).
  - moved all API URL generation logic & API response handling logic to separate classes
  - created ApiTestConfig class to hold API test options (and documentation for properties)
  - remove unnecessary IntegrationTestCase properties and methods (including apiNotToCall/apiToCall)
  - remove as many unnecessary API test options as possible (including abandonedCarts, hackDeleteRangeArchivesBefore)
  - rename checkRequestResponse to assertApiResponseHasNoError
@diosmosis diosmosis closed this Jul 26, 2014
@diosmosis diosmosis deleted the tests_refactor branch July 26, 2014 14:17
@diosmosis diosmosis restored the tests_refactor branch July 26, 2014 14:17
@diosmosis
Copy link
Member Author

Closed by accident

@diosmosis diosmosis reopened this Jul 26, 2014
diosmosis and others added 7 commits August 1, 2014 15:19
…/angularjs/scripts/install-ubuntu.sh to remove random travis failure.
…ld really introduce a model for sitesManager to not having directly to expose methods
…API. I tried to write a test for this but was not able to since lots of IntegrationTest magic
@mattab mattab added this to the Piwik 2.5.0 milestone Aug 1, 2014
$fieldsToRemove = @$this->params['xmlFieldsToRemove'];
}

$fieldsToRemove[] = 'idsubdatatable'; // TODO: had testNotSmallAfter, should still?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave the testNotSmallAfter test as I think it may detect bug in some cases

mattab pushed a commit that referenced this pull request Aug 1, 2014
Integration Tests refactor (move API testing logic to separate classes)
@mattab mattab merged commit 112bda2 into master Aug 1, 2014
@mattab
Copy link
Member

mattab commented Aug 1, 2014

looks good!

@diosmosis diosmosis deleted the tests_refactor branch August 10, 2014 14:10
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.

None yet

3 participants