Skip to content
This repository was archived by the owner on Dec 19, 2019. It is now read-only.

[263] API-functional tests #264

Conversation

VoronoyAlexandr
Copy link
Contributor

@VoronoyAlexandr VoronoyAlexandr commented Nov 24, 2018

Description (*)

This PR add API-function tests which cover negative scenarios

Fixed Issues (if relevant)

  1. Test coverage for PR-226 #263: Test coverage for PR-226

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

email: "[email protected]"
message: "Lorem Ipsum"
}
recipients: [
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like two elements of recipients will be enough

Copy link
Contributor

Choose a reason for hiding this comment

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

No
It's ok
Check app/code/Magento/SendFriend/etc/config.xml:15

use Magento\Catalog\Api\Data\ProductTierPriceExtensionFactory;
use Magento\Catalog\Api\Data\ProductExtensionInterfaceFactory;

\Magento\TestFramework\Helper\Bootstrap::getInstance()->reinitialize();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to have this fixture if we already have same fixture in Magento code base?

$this->expectException(\Exception::class);
$this->expectExceptionMessage("You can't send messages more than {$sendFriend->getMaxSendsToFriend()} times an hour.");

for ($i = 0; $i <= $sendFriend->getMaxSendsToFriend() + 1; $i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So, if value will be 10000 we will have huge cycle in tests

Copy link
Contributor

Choose a reason for hiding this comment

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

It is not clear so far how this 10000 can appear :).

*/
private $dataObjectFactory;

protected function setUp()
Copy link
Contributor

Choose a reason for hiding this comment

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

Pls, remove unused properties

/**
* @magentoApiDataFixture Magento/SendFriend/_files/product_simple.php
*/
public function testSendWithoutRecipentsName()
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like for validation w can use dataProviders instead of creating separate methods


/**
* @magentoApiDataFixture Magento/SendFriend/_files/product_simple.php
* @magentoApiDataFixture Magento/SendFriend/Fixtures/sendfriend_configuration.php
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally, we should use the approach described in https://github.com/magento/graphql-ce/issues/167
Pls, link this temporal solution with the #167

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I understand I can skip this comment. Just leave todo.

@naydav
Copy link
Contributor

naydav commented Jan 23, 2019

Hello @VoronoyAlexandr,
We still need some changes to finish the PR, will you continue progress?
Thank you

Please let me know if you need any assistance.

@VitaliyBoyko
Copy link
Contributor

Hi
Closing due inactivity.

@ghost
Copy link

ghost commented Feb 6, 2019

Hi @VoronoyAlexandr, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants