Skip to content
This repository has been archived by the owner on Apr 29, 2019. It is now read-only.

Eliminate usage of Zend_Mail from Magento 2 Open Source #136

Open
wants to merge 3 commits into
base: 2.3.1-develop
Choose a base branch
from
Open

Eliminate usage of Zend_Mail from Magento 2 Open Source #136

wants to merge 3 commits into from

Conversation

mhauri
Copy link
Contributor

@mhauri mhauri commented Jun 2, 2018

Description

Fixed Issues (if relevant)

  1. Eliminate usage of Zend_Mail from Magento 2 Open Source #72 Eliminate usage of Zend_Mail from Magento 2 Open Source

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)

@magento-cicd2
Copy link

magento-cicd2 commented Jun 2, 2018

CLA assistant check
All committers have signed the CLA.

@magento-cicd2
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@@ -71,18 +71,18 @@ protected function mockModel($filesystem = null)

$this->objectManager->get(\Magento\Framework\App\State::class)->setAreaCode('frontend');

$this->model->expects($this->any())->method('_getMail')->will($this->returnCallback([$this, 'getMail']));
$this->model->expects($this->any())->method('_getMail')->will($this->returnCallback([$this, 'getMessage']));
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 _getMail() method doesn't exist anymore. It shouldn't be necessary here then.

Copy link
Contributor Author

@mhauri mhauri Jun 7, 2018

Choose a reason for hiding this comment

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

Thank you @buskamuza for catching this, I overlooked it. Will fix it asap.
This test seems to be obsolete, am I right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm... probably the test itself still makes some sense. But anything related to $this->mail does not.

@mhauri
Copy link
Contributor Author

mhauri commented Jun 17, 2018

Hi @buskamuza I adapted the test, it seems that ->setMethods(['mockableMethod']) is required to load the mock correctly even when the method doesn't exist and is only for mocking. But I removed all the references to _getMail.

@mhauri
Copy link
Contributor Author

mhauri commented Jul 3, 2018

Hi @buskamuza anything else I can do which helps in processing this PR?

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

Successfully merging this pull request may close these issues.

3 participants