Skip to content

Test suite turns green on PHP 8.0 - #514

Merged
bishopb merged 24 commits into
masterfrom
chore/77569/pivot-toward-php-8
Feb 19, 2021
Merged

bishopb merged 24 commits into
masterfrom
chore/77569/pivot-toward-php-8

Conversation

@bishopb

@bishopb bishopb commented Feb 18, 2021 •

Copy link
Copy Markdown

Description of the change

In this change, we set a minimum version of PHP 8.0, update dependencies to the latest stable versions, remove tests that apply only to earlier engine versions, and compensate for changes in the PHP engine and dependents.

This is a precursor to expanding the library to leverage language improvements in PHP 8. However, no changes were made beyond getting the tests to turn green on PHP 8.

Typical kinds of changes:

  • Remove tests and scenarios that apply to PHP versions less than 8
  • Add type signatures where necessary to satisfy LSP
  • Replace deprecated methods from PHPUnit

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • "Ready for review" label attached to the PR and reviewers mentioned in a comment
  • Changes have been reviewed by at least one other engineer
  • Issue from task tracker has a link to this pull request

@bishopb
bishopb requested review from mrunalk and waltjones February 18, 2021 03:15
@waltjones

Copy link
Copy Markdown
Contributor

👍 Nice work. Looks like CI is still enabled for 7.x?

Bishop Bettini added 24 commits February 18, 2021 12:59
Draw the line in the sand early: we're pivoting toward PHP 8.
This includes support for PHPUnit 9, which wants the phpunit config
file to validate against a newer schema. No features are lost in
this translation.
Nice to have a docker environment ready to go to test our changes.
This particular test makes no assertions, when it should if it's
going to be a test. Needs further analysis, but for now we shall
mark it explicitly as being skipped. (Transitioning it from "risky"
to "skipped".)
Necessitated by PHPUnit 9 removing assertions for values of
protected and private members, add a method to inspect the given
config.
When creating mocks based on classes implementing `\Serializable`,
a type signature error was thrown because the mock uses the
signature of the interface for unmocked methods, which differs
from the signature of our classes.

This commit fixes the failures, by aligning our method with the
`\Serializable` interface for the `unserialize` method, but that's
only half the problem: the `serialize` method doesn't always
return a string, as it's required by the interface. A later fix
will address that.
For performance reasons, PHP 7.4 implemented a new INI setting
that doesn't capture the local variables and set its default value
to off. Consequently, if one requests local var capture AND uses
the engine in its default configuration, then it will silently
fail (no vars captured, when vars exist to be captured).

The fix adopted here is to explicitly tell the engine to capture
variables in the trace when the user requests local var capture.
Also, improve name of one test to match its covered functionality.
PHPUnit assumes that the file name matches the class name inside
the file when running a single file test.
Monolog 1 is no longer a dependency, so no need to have tests for
it.
Appears to copy the behavior of the verbosity test.
Misalignment deprecated in PHPUnit 9.
When consumers use Monolog directly, they may use the Monolog-
provided Rollbar handler to communicate with Rollbar. See also:
https://github.com/Seldaek/monolog/blob/main/src/Monolog/Handler/RollbarHandler.php
It's deprecated in PHPUnit 10, and we can use a withConsecutive call
to obtain the same behavior with a little less brittleness. While in
there, also remove the unabashed exception swallowing, as it could
lead to false positives (as it did while converting this code).
Instead, we'll specifically mark cases where an exception is expected
just as we would in other test cases.
@bishopb
bishopb force-pushed the chore/77569/pivot-toward-php-8 branch from 40daea2 to 99ae8d0 Compare February 18, 2021 17:59

@mrunalk mrunalk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great! LGTM!

@bishopb
bishopb merged commit dfe5cf2 into master Feb 19, 2021
@bishopb
bishopb deleted the chore/77569/pivot-toward-php-8 branch February 19, 2021 00:17
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.

3 participants