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

Behavioral tests for Manufacturer commands #14145

Merged
merged 11 commits into from Aug 26, 2019

Conversation

@zuk3975
Copy link
Contributor

commented Jun 10, 2019

Questions Answers
Branch? develop
Description? Adds behat tests for manu facturer create/edit/delete actions
Type? refacto
Category? TE
BC breaks? no
Deprecations? no
Fixed ticket? #14480
How to test? tests can be executed by command: ./vendor/bin/behat -c tests/Integration/Behaviour/behat.yml -s manufacturer

This change is Reviewable

@zuk3975 zuk3975 requested a review from PrestaShop/prestashop-core-developers as a code owner Jun 10, 2019
@zuk3975 zuk3975 changed the title [WIP] Behavioral tests for Manufacturer commands [WIP] Behavioural tests for Manufacturer commands Jun 20, 2019
@matks matks added the migration label Jun 24, 2019
@zuk3975 zuk3975 changed the title [WIP] Behavioural tests for Manufacturer commands Behavioural tests for Manufacturer commands Jun 25, 2019
@zuk3975 zuk3975 force-pushed the zuk3975:behat/manufacturer branch from eff2364 to 2fb784a Jun 25, 2019
@zuk3975 zuk3975 changed the title Behavioural tests for Manufacturer commands [WIP] Behavioural tests for Manufacturer commands Jul 16, 2019
…for bulk status-toggle/delete actions
@zuk3975 zuk3975 changed the title [WIP] Behavioural tests for Manufacturer commands Behavioural tests for Manufacturer commands Jul 17, 2019
@zuk3975 zuk3975 changed the title Behavioural tests for Manufacturer commands Behavioral tests for Manufacturer commands Jul 17, 2019
@matks matks added this to the 1.7.7.0 milestone Jul 18, 2019
@matks

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

@zuk3975 Travis is not happy about the behat tests, can you check ?

*/
public function editManufacturerWithDefaultLang($reference, TableNode $node)
{
$manufacturer = SharedStorage::getStorage()->get($reference);

This comment has been minimized.

Copy link
@sarjon

sarjon Jul 24, 2019

Member

Could be typehinted for autocomplete.

@reset-database-before-feature
#./vendor/bin/behat -c tests/Integration/Behaviour/behat.yml -s manufacturer
Feature: Manufacturer management
As a employee

This comment has been minimized.

Copy link
@sarjon

sarjon Jul 24, 2019

Member
Suggested change
As a employee
As an employee
#./vendor/bin/behat -c tests/Integration/Behaviour/behat.yml -s manufacturer
Feature: Manufacturer management
As a employee
I must be able to correctly add, edit and delete manufacturer

This comment has been minimized.

Copy link
@sarjon

sarjon Jul 24, 2019

Member
Suggested change
I must be able to correctly add, edit and delete manufacturer
I must be able to add, edit and delete manufacturers
And manufacturer "shoeman" should be disabled

Scenario: Enable and disable manufacturer status
Given manufacturer "shoeman" should be disabled

This comment has been minimized.

Copy link
@sarjon

sarjon Jul 24, 2019

Member

I think Given manufacturer "shoeman" is disabled would be more correct.

When I disable manufacturer "shoeman"
Then manufacturer "shoeman" should be disabled

Scenario: Enabling multiple manufacturers in bulk action

This comment has been minimized.

Copy link
@sarjon

sarjon Jul 24, 2019

Member
Suggested change
Scenario: Enabling multiple manufacturers in bulk action
Scenario: Enabling and disabling multiple manufacturers in bulk action
When I disable multiple manufacturers: "baller, rocket" using bulk action
Then manufacturers: "baller, rocket" should be disabled

Scenario: Deleting manufacturer right after changing its name

This comment has been minimized.

Copy link
@sarjon

sarjon Jul 24, 2019

Member

Is it some kind of special case when you delete "just after renaming it"?

This comment has been minimized.

Copy link
@zuk3975

zuk3975 Jul 24, 2019

Author Contributor

Yeah, it is just what matk suggested to do, to avoid a bit redudnat/short scenarios. #14132 (comment)

This comment has been minimized.

Copy link
@sarjon

sarjon Jul 24, 2019

Member

Deleting manufacturer alone is a valid scenario imo, but this works as well.

@zuk3975

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2019

@matks this one could also be merged after making travis happy 😄

@matks
matks approved these changes Aug 20, 2019
@matks

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2019

Thank you @zuk3975

@matks matks merged commit 33e39a2 into PrestaShop:develop Aug 26, 2019
2 of 3 checks passed
2 of 3 checks passed
code-review/reviewable 4 files, 6 discussions left (sarjon, zuk3975)
Details
PrettyCI Code formatting
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.