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

Refactor controller tests #26437

Merged
merged 6 commits into from Oct 27, 2021

Conversation

jolelievre
Copy link
Contributor

@jolelievre jolelievre commented Oct 26, 2021

Questions Answers
Branch? develop
Description? Refactor the controller tests, too many things were happening inside the setUp method
Which makes the base class difficult to reuse
The generic behaviour remains the same but everything was split into separate methods that can be used when needed
Also separate into two abstract classes depending if you need to test the grid only or the form with it (not possible to test the form without the grid though)
Type? refacto
Category? TE
BC breaks? no
Deprecations? no
Fixed ticket? ~
How to test? CI tests green
Possible impacts? Improved efficiency in testing controllers

This change is Reviewable

…and each action is now separated in a dedicated test function instead of being all done in the setup
…ral times which helps debugging, no need to clean the DB at each time
@jolelievre jolelievre requested a review from a team as a code owner October 26, 2021 18:07
@prestonBot prestonBot added develop Branch Refactoring Type: Refactoring labels Oct 26, 2021
@jolelievre jolelievre added the TE Category: Tests label Oct 26, 2021
@jolelievre jolelievre added this to the 8.0.0 milestone Oct 26, 2021
PierreRambaud
PierreRambaud previously approved these changes Oct 27, 2021
tegbessou
tegbessou previously approved these changes Oct 27, 2021
Copy link
Contributor

@tegbessou tegbessou left a comment

Choose a reason for hiding this comment

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

Very nice job, I have just seen a problem with a typo! Thank you!

public function handleFor($id, FormInterface $form)
{
return $this->formHandler->handleFor($form, $id);
return $this->formHandler->handleFor($id, $form);
Copy link
Contributor

Choose a reason for hiding this comment

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

😮 woah this was probably introducing bugs before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, because it was not used The edition was never tested until now

@PierreRambaud PierreRambaud merged commit c84d096 into PrestaShop:develop Oct 27, 2021
@PierreRambaud
Copy link
Contributor

Thank you @jolelievre

@jolelievre jolelievre deleted the refacto-controller-tests branch June 9, 2022 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
develop Branch Refactoring Type: Refactoring TE Category: Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants