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

add Dont/ToString #19

Closed
wants to merge 2 commits into from
Closed

Conversation

OskarStark
Copy link
Contributor

Fixes #18

@OskarStark
Copy link
Contributor Author

The problem here is to test the exception when an exception is not allowed to be thrown 🤔

@OskarStark OskarStark changed the title add Dont/__toString() add Dont/ToString Dec 17, 2019
@Ocramius
Copy link
Member

@OskarStark bump composer.json to 7.4 and you can add a test

@OskarStark
Copy link
Contributor Author

Done ✅

@OskarStark OskarStark force-pushed the dont-__toString branch 2 times, most recently from 9e1a97d to c95d945 Compare December 17, 2019 10:32
@OskarStark
Copy link
Contributor Author

I bumped Travis too, another option would be to skip the test for lower php versions

@Ocramius
Copy link
Member

another option would be to skip the test for lower php versions

Meh, feasible, but still a hack

*/
public static function fromAttemptedToString($object) : self
{
if (! is_object($object)) {
Copy link
Member

Choose a reason for hiding this comment

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

How can this particular scenario be reached? Also, contradiction between docblock and this check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbh I copy pasted this from DontClone

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed cfb3dda

ERROR;

/**
* @param object $object
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this to the type declaration

@OskarStark
Copy link
Contributor Author

Meh, feasible, but still a hack

I am fine with 7.4 only 👍

@Ocramius
Copy link
Member

Hmm, testWillThrowOnNonObject was still being executed?

@Ocramius
Copy link
Member

@OskarStark I think we need to see what the uncovered code is... Potentially false positive in infection

@OskarStark
Copy link
Contributor Author

Any idea how I can achieve that?

@OskarStark
Copy link
Contributor Author

@Ocramius can you please help me fixing this one and get it merged?

@Ocramius
Copy link
Member

Ocramius commented May 8, 2020

@OskarStark try running the mutation test framework locally (or change it to print the log to php://stderr, so we can see the output in CI)

@asgrim asgrim changed the base branch from master to 1.2.x December 7, 2020 09:53
@asgrim asgrim removed this from the 1.2.0 milestone Dec 7, 2020
@afilina afilina mentioned this pull request Jan 6, 2021
@Ocramius Ocramius added this to the 1.3.0 milestone Jan 7, 2021
@Ocramius Ocramius self-assigned this Jan 7, 2021
@Ocramius
Copy link
Member

Ocramius commented Jan 7, 2021

Ported and merged in #32 👍

@Ocramius Ocramius closed this Jan 7, 2021
@OskarStark OskarStark deleted the dont-__toString branch January 7, 2021 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

__toString()
3 participants