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

fix(integrity): expect pkg mgmt files #490

Closed
wants to merge 8 commits into from
Closed

fix(integrity): expect pkg mgmt files #490

wants to merge 8 commits into from

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Sep 12, 2023

backport of #488

@blizzz blizzz added this to the Nextcloud 27.1.0 milestone Sep 12, 2023
@blizzz
Copy link
Member Author

blizzz commented Sep 12, 2023

Tested on an instance that was having the issue, and it fixes it.

@come-nc
Copy link
Collaborator

come-nc commented Sep 12, 2023

CI is not happy.

When doing "composer i" I end up with files removed from git, is that expected?

@blizzz
Copy link
Member Author

blizzz commented Sep 13, 2023

CI is not happy.

When doing "composer i" I end up with files removed from git, is that expected?

Sounds strange, what would it want to remove?

The makefile (and also tests) differ from master, it surely is worth to streamline them as well.

@blizzz
Copy link
Member Author

blizzz commented Sep 13, 2023

Pushed minimal adjustments to the tests.

@blizzz blizzz force-pushed the backport/488/stable27 branch 4 times, most recently from b196e8f to 008e644 Compare September 13, 2023 13:04
@blizzz
Copy link
Member Author

blizzz commented Sep 13, 2023

"minimal"… tbc

@blizzz blizzz force-pushed the backport/488/stable27 branch 2 times, most recently from ebfe0a0 to ee0fd72 Compare September 13, 2023 17:47
blizzz and others added 6 commits September 13, 2023 21:21
@blizzz
ci(config): test against related NC version
70ca39d 
- tests NC27
- bump humbug/box

Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
fix(build): add updater.phar with clean version
c9e29c7 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
deps(php): fix php platform and update deps
81da230 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
ci(deps): set platform and update behat
6e8c8f3 
Signed-off-by: Arthur Schiwon <[email protected]>
@skjnldsv @blizzz
fix: psalm
f9cd878 
Signed-off-by: John Molakvoæ <[email protected]>
@blizzz
ci(deps): bump symfony/console to 6.0
2fbf2cd 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
Copy link
Member Author

blizzz commented Sep 13, 2023

totally nuts. Why don't I copy over test definition from master? It succeeds even on 8.2 for unknown reasons, but it does not test against 27. Still might give it a try tomorrow, this is frustration. In this repo we almost never have PRs against stable branches…

@blizzz blizzz mentioned this pull request Sep 13, 2023
Merged
come-nc
come-nc requested changes Sep 14, 2023
View reviewed changes
tests/features/master.feature
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels weird to stop testing that, support for daily channel is a feature of the updater.

tests/features/stable19.feature
@@ -1,32 +0,0 @@
Feature: CLI updater - stable19 base

Scenario: Update is available - 19.0.0 beta 3 to 19.0.0 beta 4
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, this tests beta support I think

tests/features/stable20.feature
@@ -1,54 +0,0 @@
Feature: CLI updater - stable20 base

Scenario: Update is available - 20.0.0 beta 1 to 20.0.0 RC 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This tests beta to RC

tests/features/stable20.feature
And maintenance mode should be off
And upgrade is not required

Scenario: Update is available but unexpected folder found - 20.0.7 to 20.0.8
Copy link
Collaborator

Choose a reason for hiding this comment

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

unexpected folder here.

All these tests should be kept IMO since they test features of the updater.

@come-nc
Copy link
Collaborator

come-nc commented Sep 14, 2023

Also why the symfony/console bump in stable27 that was not done in master first?

@come-nc
Copy link
Collaborator

come-nc commented Sep 14, 2023

CI is not happy.
When doing "composer i" I end up with files removed from git, is that expected?

Sounds strange, what would it want to remove?

The makefile (and also tests) differ from master, it surely is worth to streamline them as well.

My composer was outdated, this does not happen anymore.

@blizzz
Copy link
Member Author

blizzz commented Sep 14, 2023

Also why the symfony/console bump in stable27 that was not done in master first?

Tests result complained about PHP 8.2 incompatibilities, so that was a try. That's also why I wonder why they work in master.

@blizzz
Copy link
Member Author

blizzz commented Sep 14, 2023

As discussused wiht @come-nc we'll use #492 for an immediate fix with green CI and he'll do an follow up with freshing up tests. The fundamental techniques of the updater are still tested, there is only very little interaction with business logic, and master still succeeds.

@blizzz blizzz closed this Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc requested changes

@juliusknorr juliusknorr juliusknorr approved these changes

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

Assignees
No one assigned
Labels
3. to review
Projects
None yet
Milestone
Nextcloud 27.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@blizzz @come-nc @AndyScherzinger @juliusknorr @skjnldsv