API Rescue Master Branch PR: TestCase classes should be abstract to prevent warnings

[GuySartorelli]

Rescues #7907

Parent issue

• Rescue master branch commits #10350

[emteknetnz]

I don't like this change, there's no abstract methods in SapphireTest so making this an abstract class just to prevent warnings (of which there are none??) seems like an anti-pattern

We should probably close this unless there's a good reason to keep it

[GuySartorelli]

prevent warnings (of which there are none??)

There are none because of the hacky workaround which is being removed in this same change:

// We cannot run the tests on this abstract class.
if (static::class == __CLASS__) {
    $this->markTestSkipped(sprintf('Skipping %s ', static::class));
}

abstract doesn't only mean "There are methods here which we want you to implement in your own class" - it can also be used as a way to identify classes which simply should never be instantiated - they're meant to be subclassed, not used directly. That's exactly the use case here.

[emteknetnz]

There are none because of the hacky workaround which is being removed in this same change

Ah OK, that makes more sense

used as a way to identify classes which simply should never be instantiated - they're meant to be subclassed, not used directly

Sounds good