[TASK] Add tests for the exceptions

[oliverklee]

Part of #757

[JakeQZ]

I like the running theme of amusing and bizarre messages used in the tests. (Some hosts give me tea in the morning when coffee is expected; YMMV.)

I think one of the test methods (repeated 4 times) is conflated, and that there should be two separate test methods: one for the message, and one for the line number.

I'm also wondering about the duplication. I think it's right to also test that the extended classes' behaviour match the base (where that is expected). But I wonder if it's possible to have a base class or trait for the TestCase to provide the common tests - if that would work with PHPUnit which I know relies on @test in the DocBlock.

[JakeQZ]

Ditto (re two separate tests and naming).

[JakeQZ]

Ditto.

[JakeQZ]

Ditto.

[JakeQZ]

GitHub lost my comment here. The various 'dittos' refer to this.

I think this is conflating two tests, one that the line number is included, and one that the message is included when a line number is provided. Also the name is confusing. I'd suggest

• getMessageWithLineNumberProvidedIncludesMessage and
• getMessageWithLineNumberProvidedIncludesLineNumber.

[oliverklee]

Thanks, I mixed up the tests names indeed. Fixed.

As far as the message provided to the constructor is concerned, we already have the getMessageReturnsMessageProvidedToConstructor tests for cases where the exception actually uses the message provided to the constructor.

[JakeQZ]

True, but we don't have a test to confirm the message when a line number argument is passed.

[JakeQZ]

I added back (roughly) a key comment that GitHub lost.

[coveralls]

coverage: 50.527% (+1.4%) from 49.156%
when pulling d1b22cb on task/exception-tests
into 537f5e8 on main.

[oliverklee]

I like the running theme of amusing and bizarre messages used in the tests. (Some hosts give me tea in the morning when coffee is expected; YMMV.)

❤️

[oliverklee]

I'm also wondering about the duplication. I think it's right to also test that the extended classes' behaviour match the base (where that is expected). But I wonder if it's possible to have a base class or trait for the TestCase to provide the common tests - if that would work with PHPUnit which I know relies on @test in the DocBlock.

For class testing hierarchies, we could add an assertInstanceOf that tests that the subclass extends the base class, and rely on the tests of the base class.

I'll add tests for the instanceOf. As far as the behavior is concerned, I'd prefer to have the tests duplicated. This makes the test easier to understand, and it will allow us to refactor more easily.

[JakeQZ]

As far as the behavior is concerned, I'd prefer to have the tests duplicated. This makes the test easier to understand, and it will allow us to refactor more easily.

Duplicated code shrieks at me. It's much less maintainable, It's fine for now, but I would like to come up with a way of avoiding it.

[JakeQZ]

Added #919 to cover possible clean-up.