Skip to content

Test to illustrate thrown exception (new domain entity)#13

Open
ElGigi wants to merge 10 commits into
atlasphp:0.xfrom
ElGigi:0.x
Open

Test to illustrate thrown exception (new domain entity)#13
ElGigi wants to merge 10 commits into
atlasphp:0.xfrom
ElGigi:0.x

Conversation

@ElGigi

@ElGigi ElGigi commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

Issue explain here:
#10

@ElGigi

ElGigi commented Apr 25, 2019

Copy link
Copy Markdown
Contributor Author

I added a fix

@ElGigi

ElGigi commented Apr 30, 2019

Copy link
Copy Markdown
Contributor Author

Any news? Thanks.

Comment thread tests/ValueObjectTest.php Outdated


// Test new entity
$newFakeEntity = new Fake(new Email('fake@example.com'),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your changes are related to the EntityHandler but this is the test class for a value object. Is this correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes because it's on the persist of a ValueObject.

@froschdesign froschdesign Apr 30, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Two suggestions for the test:

  1. Create a separate test method and use a name that describes the current testing process.
  2. At the moment the test did not perform any assertions, that should be changed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done ;)

@ElGigi

ElGigi commented Jun 12, 2019

Copy link
Copy Markdown
Contributor Author

Any news? Project abandoned?
Thanks.

@froschdesign

Copy link
Copy Markdown

@ElGigi
I think @pmjones is busy or has to make money. So nothing special, just normal life.

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.

2 participants