Use expectException(Message) instead of annotations

[olvlvl]

Q	A
Type	improvement
BC Break	no

Summary

The @expectedException and @expectedExceptionMessage annotations are deprecated. They will be removed in PHPUnit 9. This PR replaces @expectedException and @expectedExceptionMessage annotations with expectedException() and expectedExceptionMessage().

My only concern is with OutTest because builder->out() is throwing the exception, not builder->getPipeline().

[malarzm]

My only concern is with OutTest because builder->out() is throwing the exception, not builder->getPipeline().

I think that getPipeline() call could be removed in this case

[malarzm]

Personally I prefer putting expectException as a first thing in a test but I'm not sure what's the preference of us as a team :)

[olvlvl]

We put the expectException() just before the thing that is expected to throw an exception, because some exceptions such as \InvalidArgumentException can be thrown in many cases. Let me know what to do.

[alcaeus]

Your usage is correct - test setup should not trigger the expected exception by accident. Please leave it as is 👍

[malarzm]

Awesome, thank you!

[greg0ire]

Great! I'd squash both commits together, since it's very unlikely that we would revert one and not the other.

[olvlvl]

@greg0ire Do you want me to do the squash, or will you squash+merge?

[malarzm]

@olvlvl please squash, we're not using squash+merge as it's not doing an actual merge (or it wasn't doing this last time we were checking)

[greg0ire]

Also, we would rather have you be in charge of the final commit message ;)

[olvlvl]

Squashed.

[greg0ire]

Thanks @olvlvl !