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

Retrofit existing code with unit tests #10

Merged
merged 7 commits into from
May 13, 2020

Conversation

agentS
Copy link
Contributor

@agentS agentS commented May 13, 2020

This PR adds unit tests for the segment-based adapter, the purchase history-based adapter, and the equal weight decorator.
The issues we ran into when trying to unit test the ETL mechanism are documented on a wiki page.

Furthermore, this PR cleans up the code for unit testing and according to the suggestions of the PHP Stan tool.

agentS added 7 commits May 6, 2020 14:23
This commit introduces unit tests for the purchase history-based
adapter. In order to enable those unit tests, the call to the factory
for obtaining the user ID has been moved into a component provided via
DI.
…ch into 32-retrofit-existing-code-with-unit-tests
This commit adds unit test for the equal weight decorator using both
the purchase history-based adapter and the segment-based adapter
as child adapters. Both sequences of adding the adapters are tested.
This commit cleans up the code following the messages generated by
PHP Stan.
This commit cleans up the code following the hints generated by
PHP Stan.
Copy link
Contributor

@PatrikHubster PatrikHubster left a comment

Choose a reason for hiding this comment

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

Create methods for code duplication if we have time.

$queryRed = array ( 'multi_match' => array ( 'query' => 'red', 'type' => 'cross_fields', 'operator' => 'and', 'fields' => array ( 0 => 'attributes.name^4', 1 => 'attributes.name.analyzed', 2 => 'attributes.name.analyzed_ngram', 3 => 'attributes.manufacturer_name^3', 4 => 'attributes.manufacturer_name.analyzed', 5 => 'attributes.manufacturer_name.analyzed_ngram', 6 => 'attributes.color', 7 => 'attributes.carClass', ), ), );
$expectedPurchaseHistoryBoostedQueryRed = array ( 'function_score' => array ( 'query' => array ( 'multi_match' => array ( 'query' => 'red', 'type' => 'cross_fields', 'operator' => 'and', 'fields' => array ( 0 => 'attributes.name^4', 1 => 'attributes.name.analyzed', 2 => 'attributes.name.analyzed_ngram', 3 => 'attributes.manufacturer_name^3', 4 => 'attributes.manufacturer_name.analyzed', 5 => 'attributes.manufacturer_name.analyzed_ngram', 6 => 'attributes.color', 7 => 'attributes.carClass', ), ), ), 'functions' => array ( 0 => array ( 'filter' => array ( 'match' => array ( 'relations.segments' => 983, ), ), 'weight' => 6.0, ), 1 => array ( 'filter' => array ( 'match' => array ( 'relations.segments' => 963, ), ), 'weight' => 6.0, ), 2 => array ( 'filter' => array ( 'match' => array ( 'relations.segments' => 982, ), ), 'weight' => 6.0, ), 3 => array ( 'filter' => array ( 'match' => array ( 'relations.segments' => 971, ), ), 'weight' => 6.0, ), 4 => array ( 'filter' => array ( 'match' => array ( 'relations.segments' => 970, ), ), 'weight' => 6.0, ), ), 'boost_mode' => 'multiply', ), );

$orderIndexResponse = array ( 0 => array ( 'segmentId' => 983, 'segmentCount' => 1, ), 1 => array ( 'segmentId' => 963, 'segmentCount' => 1, ), 2 => array ( 'segmentId' => 982, 'segmentCount' => 1, ), 3 => array ( 'segmentId' => 971, 'segmentCount' => 1, ), 4 => array ( 'segmentId' => 970, 'segmentCount' => 1, ), );
Copy link
Contributor

Choose a reason for hiding this comment

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

The following four lines are always the same in three methods, we can create an method for doing this instead.

private function constructPurchaseHistoryAdapter(int $customerId, array $orderIndexResponse) : PurchaseHistoryAdapter {
$orderIndex = $this->getMockBuilder(OrderIndexAccessProvider::class)
->setMethods(['fetchSegments'])
->getMock();
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect usage of mocking :-)

@agentS agentS merged commit 64baf97 into master May 13, 2020
@fashxp fashxp deleted the 32-retrofit-existing-code-with-unit-tests branch June 30, 2020 13:19
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

2 participants