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.

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

PHP8 CI #23780

Merged
merged 23 commits into from
Dec 8, 2020
Merged

PHP8 CI #23780

merged 23 commits into from
Dec 8, 2020

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented Oct 29, 2020

Good to go!

The test for other DBs will follow in later PRs

@rullzer rullzer added the 2. developing Work in progress label Oct 29, 2020
@rullzer rullzer added this to the Nextcloud 21 milestone Oct 29, 2020
@MorrisJobke
Copy link
Member

[ ] Rebase once #23764 is in

Please also wait for #23834

@rullzer rullzer force-pushed the enh/ci/php8 branch 2 times, most recently from 87b3f1c to 336d919 Compare November 10, 2020 19:07
@rullzer rullzer force-pushed the enh/ci/php8 branch 2 times, most recently from dbe36da to 69a2d89 Compare November 30, 2020 15:29
@rullzer
Copy link
Member Author

rullzer commented Dec 2, 2020

There was 1 failure:

1) Test\LargeFileHelperGetFileSizeTest::testGetFileSizeViaExec with data set #1 ('/drone/src/tests/data/strängé...2).txt', 446)
Failed asserting that null is identical to 446.

/drone/src/tests/lib/LargeFileHelperGetFileSizeTest.php:70

I think this is a locale issue...
See also: https://3v4l.org/vHqno

There was 1 error:

1) Test\Mail\EMailTemplateTest::testEMailTemplateSingleButton
ValueError: Unknown format specifier ";"

/drone/src/lib/private/Mail/EMailTemplate.php:592
/drone/src/tests/lib/Mail/EMailTemplateTest.php:180

No clue about that one yet.

@rullzer
Copy link
Member Author

rullzer commented Dec 3, 2020

Ok lets review and see if we can still get this in.

@@ -108,9 +112,9 @@ public function testRun() {
$backend3 = $this->createMock(IBackend::class);
$backend4 = $this->createMock(IBackend::class);

$res6 = $this->createMock([IResource::class, IMetadataProvider::class]);
Copy link
Member

Choose a reason for hiding this comment

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

oh wait, that worked?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes... But killed in php unit 9

@rullzer
Copy link
Member Author

rullzer commented Dec 4, 2020

The psalm errors come from the fact that it doesn't support php8 properly i guess. since it doesn't know that openssl returns an object now. And the GD stuff a GdImage.

@rullzer
Copy link
Member Author

rullzer commented Dec 4, 2020

Sure now oracle is complaining...

@nickvergessen
Copy link
Member

Seems like

public function getSortedKeys($data) {
$keys = array_keys($data);
sort($keys);
return $keys;
}
doesn't sort numbers before characters anymore

@ChristophWurst
Copy link
Member

The psalm errors come from the fact that it doesn't support php8 properly i guess. since it doesn't know that openssl returns an object now. And the GD stuff a GdImage.

could it be because of build/stubs?

@rullzer
Copy link
Member Author

rullzer commented Dec 7, 2020

The psalm errors come from the fact that it doesn't support php8 properly i guess. since it doesn't know that openssl returns an object now. And the GD stuff a GdImage.

could it be because of build/stubs?

No clue why. But I chose to igrnoe it for now

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
the assertAttributeEquals got removed.
So we can't run these tests anymore.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
The comparrison on php8 return true while <php8 it is false.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
* We should pick better default method!

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@faily-bot
Copy link

faily-bot bot commented Dec 7, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 273: failure

mariadb10.4-php7.4

Show full log
There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There was 1 failure:

1) OCA\ShareByMail\Tests\ShareByMailProviderTest::testAddShareToDB
Failed asserting that 0 is identical to 1.

/drone/src/apps/sharebymail/tests/ShareByMailProviderTest.php:556

@rullzer rullzer added 3. to review Waiting for reviews enhancement and removed 2. developing Work in progress labels Dec 7, 2020
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

I like the part where we skip tests -> faster CI 🚀

Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

🦈 Bite by Bite

@guystreeter
Copy link

The PHP 8 changes for lib/private/legacy/OC_Image.php are incomplete. There are several more places where is_resource is used on an object. The loadFrom* and *resize* functions and the crop function use it and need to be modified.

@nickvergessen
Copy link
Member

You mean 8.1? See #29287 for details

@guystreeter
Copy link

If that was a response to my comment, these functions are broken in 8.0. I don't see anything in that indicating 8.1 will be any different.

@nickvergessen
Copy link
Member

Ah sorry, then I mixed it up.
Anyway please create a new bug report. Replying to a 1year old PR which was merged is not going to be successful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants