Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add support for PHP 8 #24

Merged
merged 9 commits into from
Aug 16, 2023
Merged

add support for PHP 8 #24

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3500159
add matrix tests for PHP 8.0, 8.1 and 8.2
drealecs Jul 27, 2023
eaa9783
allow required php also 8.* for 8.0, 8.1 and 8.2
drealecs Jul 27, 2023
8964a64
Merge remote-tracking branch 'origin/master' into add_php8_support
beniamin-gheorghita-payu-gpo Aug 9, 2023
eec7546
Fixed the unit tests in order to have them running on php 8.1
beniamin-gheorghita-payu-gpo Aug 10, 2023
3b4c8f9
Fixed type check on integration test
beniamin-gheorghita-payu-gpo Aug 10, 2023
595b077
Reverted wrongly removed file
beniamin-gheorghita-payu-gpo Aug 10, 2023
a88903e
Added cast for mysql count result
beniamin-gheorghita-payu-gpo Aug 11, 2023
d9f5cc5
Changed the approach for PDO returned values on tests and changed str…
beniamin-gheorghita-payu-gpo Aug 16, 2023
e4ab590
Assert method replacement and test optimization
beniamin-gheorghita-payu-gpo Aug 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/phpunit-integration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ jobs:
- ubuntu-latest
php-version:
- 7.4
- 8.0
- 8.1
- 8.2

steps:
- name: Checkout
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/phpunit-unit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ jobs:
- ubuntu-latest
php-version:
- 7.4
- 8.0
- 8.1
- 8.2

steps:
- name: Checkout
Expand Down
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you think that we should remove the platform from config (line 19), so we won't have any hard restriction on version number?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we might want to remove that "php": "7.4" platform config.
If we don't run composer as a container (that runs on various php version, usually latest), it's not needed anymore.

But I'm not sure if it bothers us if we keep it at minimum. So I think we don't need to change it now.

Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"qossmic/deptrac-shim": "^0.24"
},
"require": {
"php": "^7.4",
"php": "^7.4 || ~8.0.0 || ~8.1.0 || ~8.2.0",
"symfony/yaml": "^5.0",
"ext-json": "*",
"myclabs/php-enum": "^1.7.1"
Expand Down
4 changes: 2 additions & 2 deletions lib/Application/Observer/Progress.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ final class Progress implements ProcessObserverInterface
private const FULL = "\u{2588}";
private const EMPTY = "\u{2591}";

private static $output = STDERR;
private $output = STDERR;

private $total = 0;
private $startedAt;
Expand Down Expand Up @@ -144,7 +144,7 @@ private function &getAnonymizationType($key): array

private function output($string): void
{
fwrite(self::$output, $string);
fwrite($this->output, $string);
}

private function round($microseconds, $decimals = 2, $pad = 6)
Expand Down
4 changes: 2 additions & 2 deletions tests-integration/AfterScriptRunTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ public function testTruncatedTable(): void
$stmt = self::$destination->query($query);
$destRow = $stmt->fetch(PDO::FETCH_ASSOC);

$this->assertGreaterThan(0, $sourceRow['nr']);
$this->assertSame('0', $destRow['nr']);
$this->assertGreaterThan(0, (int)$sourceRow['nr']);
$this->assertSame(0, (int)$destRow['nr']);
drealecs marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
7 changes: 6 additions & 1 deletion tests/AnonymizerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ final class AnonymizerTest extends TestCase
/** @var ObserverInterface|MockObject */
private $observerMock;

public const TMP_FILE_PATH = "/tmp/testData";

public function setUp(): void
{
Expand Down Expand Up @@ -286,8 +287,12 @@ public function testRun(): void

private function makeLineStream(array $lines)
{
$fp = fopen('data://text/plain;base64,' . base64_encode(implode("\n", $lines)), 'ab+');

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think this code is problematic for the input stream and it works well.
But for the output stream is not working, as data wrapper is not writable.
And maybe that's helpful for the future, as the STDIN input is similar, not writable and we can generate a proper stream that will issue a warning in case app tries to write to input.

I would suggest to:

  • keep the function as it was but rename it to makeInputStream() and keep use it for $inputStream.
  • create a new function makeOutputStream() that just fopen to php://memory and use it for $outputStream.

$fp = fopen(self::TMP_FILE_PATH . (string)rand(0, 999), 'w+');
fwrite($fp, implode("\n", $lines));

rewind($fp);

return $fp;
}

Expand Down
9 changes: 8 additions & 1 deletion tests/Application/Observer/ProgressTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ class ProgressTest extends TestCase
{
private Progress $sut;

public const TMP_FILE_PATH = "/tmp/testData";

public function setUp(): void
{
$this->sut = new Progress();
Expand Down Expand Up @@ -155,7 +157,7 @@ private function getProperty($name)

private function setOutputStream()
{
$fp = fopen('data://text/plain;base64,', 'ab+');
$fp = fopen(self::TMP_FILE_PATH, 'w+');
drealecs marked this conversation as resolved.
Show resolved Hide resolved
rewind($fp);

$refObject = new ReflectionObject($this->sut);
Expand All @@ -171,4 +173,9 @@ private function getOutputStream()
$refProperty->setAccessible(true);
return $refProperty->getValue($this->sut);
}

public static function tearDownAfterClass(): void
{
unlink(self::TMP_FILE_PATH);
}
}
Loading