silverstripe / cwp-recipe-kitchen-sink

The CWP kitchen sink! Includes all optional and suggested modules. Used for internal testing.
BSD 3-Clause "New" or "Revised" License
1 stars 5 forks source link

Add kitchen sink build for CWP1.9.x; ensure PHP 7.2 and 7.3 runs; ensure PHP 7.2 and 7.3 passes #31

Closed sminnee closed 4 years ago

sminnee commented 5 years ago

CWP 1.9.x is supported for longer than PHP 7.1.

Therefore we need to ensure that it supports PHP 7.2. In order to avoid the need for urgent recipe upgrades (always stressful), we should do this ASAP.

Although "recipes" were introduced in CWP 1, if there's not a kitchen sink build anywhere else we might want to set up a suitable daily build on a 1 branch of this repo.

Notes

brynwhyman commented 5 years ago

we might want to set up a suitable daily build on a 1 branch of this repo

Big +1

brynwhyman commented 5 years ago

Thoughts for doing this are:

sminnee commented 5 years ago

Which builds would be expected failures be, and why?

robbieaverill commented 5 years ago

There’s around 40-50 expected test failures in CWP 1.x, mostly around poorly mocked global state, templates etc. We fixed them all in 2.x but it took a long time

sminnee commented 5 years ago

Right, yep. You could maybe even simply ignore those tests altogether, rather than having them as allowed failures.

If we are to do that it might be worth adding PHP 7.2 builds to the individual modules' travis configs, at least for those modules that contain skipped tests.

robbieaverill commented 5 years ago

I was just thinking about this - CWP 1 doesn't have the concept of the recipe-plugin with project files etc, so the project files (e.g. mysite/*) in cwp-installer would probably have to be replicated in this repo for it to work. I have a suspicion that the cwp-installer files might also conflict with the ones in this repo as well... we will see =)

ScopeyNZ commented 5 years ago

So I've been banging my head against the wall that is travis trying to test this works.

In order to get a green build we have to go through and specifically tag tests as allowed failures. Doing this in a non-cwp-specific way we decided @group leaky annotations would be the best bet. There's no way to specify specific tests to exclude in config. We could exclude whole test files if there's a dodgy test but it didn't seem right to drop large chunks of tests because one of them was written with poor state isolation.

The problem I'm having is validating this process works correctly on travis. It probably does but I don't want to merge any @group risky PRs until I do. It's almost impossible to get a fork of framework running with travis_support on SS3. If someone's game to give it a go then I've created branches on the creative-commoners fork of this repo and framework. All branches are named pulls/3.7/integrated-ss3-builds.

Given the questionable value now that we've established 7.2/7.3 support and the fact that builds can still be "cron"ed for each module I don't really want to go down this rabbit hole any longer.

brynwhyman commented 4 years ago

I'm going to close this issue without a resolution. We have many production sites running on CWP1.9 with PHP7.2+ without fault.

Ideally we'd have an automated test suite to ensure future changes don't result in PHP-related failures, but that's proved difficult to do when we're inheriting a suite with 50+ 'expected failures' and Travis doesn't want to play ball.

Given that CWP 1.x is in limited support (Critical bugfixes and critical security fixes) the need for a nightly build across the module suite now seems less relevant too. We'll continue to rely on builds on a per-module basis.