Skip to content

Feature: Add automated Nextcloud compatibility workflow - #338

Open
kevinveenbirkenbach wants to merge 3 commits into
pulsejet:masterfrom
kevinveenbirkenbach:feature/nextcloud-compatibility-automation
Open

kevinveenbirkenbach wants to merge 3 commits into
pulsejet:masterfrom
kevinveenbirkenbach:feature/nextcloud-compatibility-automation

Conversation

@kevinveenbirkenbach

Copy link
Copy Markdown
Contributor

Summary

  • Adds a new GitHub Actions workflow to test whether the app is installable on the current Nextcloud version.
  • Starts a Compose-based test stack with Nextcloud, MariaDB, and ghcr.io/navikt/mock-oauth2-server:latest.
  • Verifies that the app can be enabled and that an OIDC login succeeds against the mock provider.
  • Automatically raises the supported Nextcloud max-version in appinfo/info.xml when the latest Nextcloud version passes compatibility checks.
  • Creates a compatibility update PR automatically when metadata changes are generated.
  • Extends the release workflow so a version bump in appinfo/info.xml on main/master triggers a release.

Validation

  • bash -n .github/nextcloud-compatibility/run.sh
  • shellcheck .github/nextcloud-compatibility/run.sh
  • actionlint .github/workflows/release.yaml .github/workflows/nextcloud-compatibility.yaml
  • git diff --check
  • act workflow_dispatch -W .github/workflows/nextcloud-compatibility.yaml -j compatibility --input nextcloud_image_tag=34 ...

The act run completed successfully against Nextcloud 34.0.1, including app activation and OIDC login.

@kevinveenbirkenbach

Copy link
Copy Markdown
Contributor Author

@pulsejet

Copy link
Copy Markdown
Owner

@kevinveenbirkenbach I'm inclined to say this is too much automation :)
Looking at the scripts it feels like maintaining the automation is going to be more work than manually doing what it's supposed to automate.

Would it be possible to trim down the PR to just the changes needed for 34 compat?

@kevinveenbirkenbach

Copy link
Copy Markdown
Contributor Author

Why? What are the disadvantages of the implementation :)

@kevinveenbirkenbach

Copy link
Copy Markdown
Contributor Author

You can also merge this one instead ;)
#337

@pulsejet

Copy link
Copy Markdown
Owner

Why? What are the disadvantages of the implementation :)

Primarily the additional 500 lines of unreadable scripting that now need to be maintained. Overall the automation is too complicated for what it offers, since the plugin should be manually tested before marking a new release anyway.

#337

Sounds good. I (incorrectly) assumed there were some additional updates needed for compat.

@kevinveenbirkenbach

Copy link
Copy Markdown
Contributor Author

That the code is unreadable I understand. Could refactor it and add some playwright tests for clarity. But why do you want to test it manual?
I test all my software in the CI fully automatic. Otherwise it would cost me to much time :)
And in cases like this it took more then a month to update the software. So the non-automatic approach seems really inefficient.

@pulsejet

pulsejet commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Broadly speaking,

  1. The maintenance burden of tests should not be more than or comparable to the application itself. Typically relevant only for integration / e2e testing.
  2. Automated tests don't replace manual QA (though LLMs might automate this some day). It's overambitious to expect traditional automation to catch all issues, e.g. visual glitches. A typical manual release will also involve consulting the changelog / upgrade guide of upstream to get a sense of any major changes or deprecations

@kevinveenbirkenbach

Copy link
Copy Markdown
Contributor Author

I disagree. I think with well written playwright tests you can catch almost all flaky behaviour and you see when UI\UX changes on a way that you need to adapt your tooling.

I teach my team every time, that the test speaks for itself. I just need to read the e2e test to understand what the software is doing and what is covered. So we have at Infinito.Nexus e2e tests in all levels.

We wouldn't had realized that the oidc_login plugin isn't updated it there wouldn't had been a test to verify the login.
So we strike for e2e 100% test coverage in all relevant components :)

For all SaaS applications which we have implemented we have autoupdate scripts which run against our tests, open a PR if a version changed and in this PR the full CI of this branch incl. all frontend playwright tests are executed.

Example for the tests in our Nextcloud role:
https://github.com/kevinveenbirkenbach/infinito-nexus-core/tree/feature/swarm-nfs/roles/web-app-nextcloud/files/playwright

And here the PR which was auto generated and showed us that the update to Nextcloud 34 wasn't possible due to the incompatibility of oidc_login with 34 :)
infinito-nexus/core#376

If you like to you can stay with the manual approach, but I can also clean up this branch and implement a structure which just needs your approval of autocreated PR's like at us :)

@HannesJo0139

HannesJo0139 commented Aug 4, 2026

Copy link
Copy Markdown

@pulsejet please keep it your way. These things should be checked carefully, especially when it's about security. We don't need a fast as possible compatibility statement - we need a reliable and secure setup. See also #339

@kevinveenbirkenbach

kevinveenbirkenbach commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

But you can automate a secure setup. I mean this is the core of TDD. I don't know why you shouldn't generate tests. Especially if you do E2E playwright testing against the current versions.

Humans make mistakes, classical tests are deterministic (if some solar flare doesn't change the charge in a transistor ;D )

Anyhow if it next time takes an month to update I would just clone it if you don't mind, because I would like to have faster release cycles :)

PS: But if you like to my offer there to refactor my automatic testing in this repository.
I just don't want to waste any resources on this if this isn't seen here as a need.

@HannesJo0139

Copy link
Copy Markdown

Your test may be deterministic but still the oidc_login is broken with current nextcloud on some setups - and your test did not find that. It only passes for your specific setup while bringing additional maintenance overhead.

@kevinveenbirkenbach

Copy link
Copy Markdown
Contributor Author

Yes but the manual approvement did neither ;D
If we know that this is a bug we could fix it with a playwright test so that it doesn't appear again.
My testing strategy is every time from big to small. As soon as I see a bug I write a test that it doesn't appear again.
I think this should be the default case in all kind of software development.
Did you actually write a piece of code in this plugin or are you just theorizing? Because if it's just a theoretical debate allow me to skip this debate and I just fork next time when a error like this appears which could be prevented by automatic testing :D
I will not try to convince people that automatic testing makes sense. If you believe it doesn't it's your believe and I'm not a missionary ;D

@pulsejet

pulsejet commented Aug 6, 2026

Copy link
Copy Markdown
Owner

@kevinveenbirkenbach seems like there's some misunderstanding here. I'm not against testing, but I am against --

  1. Overly complicated tests that are more effort to maintain than what they test. Even these have their place, but a project that is barely maintained is not it (see State of the maintainance #182)
  2. All the complex automation that automatically creates a release - is unnecessary and risky.

Typically the flow to make a new release would look like --

  1. Someone does the manual QA
  2. Creates a PR with bumped versions
  3. The test suite runs in CI and validates that nothing broke
  4. Maintainer merges + release

When (1) / (2) doesn't happen, the project is likely dead and should be archived :)

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