Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the JWKTest suite by introducing a data provider for testing multiple JWK sets and verifying public keys against expected PEM files. It also refactors the test class to use setUpBeforeClass for initialization. I have reviewed the changes and suggest using trim() when comparing PEM strings to ensure the tests are robust against potential differences in line endings or trailing whitespace.
| $this->assertEquals( | ||
| file_get_contents(__DIR__ . '/data/' . $pubkeyFile), | ||
| $publicKey | ||
| ); |
There was a problem hiding this comment.
Comparing PEM strings directly can be brittle due to differences in line endings (CRLF vs LF) or trailing newlines across different operating systems and OpenSSL versions. It is recommended to trim() both the expected and actual strings to make the test more robust and avoid failures caused by trailing whitespace.
$this->assertEquals(
trim(file_get_contents(__DIR__ . '/data/' . $pubkeyFile)),
trim($publicKey)
);
Adds a test for the fix in #628