New module for testing edgecases

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS

https://www.silverstripe.org

BSD 3-Clause "New" or "Revised" License

721 stars 821 forks source link

New module for testing edgecases #9712

Open brynwhyman opened 4 years ago

brynwhyman commented 4 years ago

Overview

Maintainers occasionally come across edge-cases in features where custom code is required to enable writing Behat / functional tests. Adding this code to frameworktest is an option, but that module is littered with a range of code that isn't strictly in service of enabling features for testing purposes. Marking objects as TestOnly is also an unsuitable solution for Behat testing.

We'd like to look at creating a new module which could hold objects for enabling testing of edge-case features, and other custom code that wouldn't make sense to go into frameworktest.

ACs

Approach is evaluated to confirm that it's a sensible path forward
Module is created with a clear README of how it is to be used, including version control rules, etc.
This module should not become a dev dependency for core modules, instead it should be installed directly in relevant CI builds

brynwhyman commented 4 years ago

An example where this module would be used: https://github.com/silverstripe/silverstripe-admin/issues/1122

sminnee commented 4 years ago

Another way to frame this is "we're able to provide code to support test fixtures specifically for behat / end-to-end tests on a per-module basis"

Providing a separate package that is installed by travis but not require-dev the proposed solution here, which assumes:

Dependencies for Behat testing are not listed as dev dependencies (presumably to stop the dev dependencies from being too bloated?)
Test fixtures for Behat testing can't be bundled as TestOnly objects (like we do with PHPUnit tests) so they need to be supplied as a separate composer package (which is perhaps fine for now)

ScopeyNZ commented 4 years ago

So a silverstripe/test-fixtures module? I feel that if you're writing Behat fixture support or feature support then it's fine for that to live in the module itself (eg. https://github.com/dnadesign/silverstripe-elemental/tree/4/tests/Behat/Context), but we never really had to dive deep into stubbing issues - we were stubbing config, not code.

I don't like the idea of having a module that the Behat tests rely on, but isn't included in require-dev, as contributors need to be informed of this extra requirement when they cause Behat failures that need to be fixed.

Can't we just ignore tests/behat in the manifest while we're not serving the project for behat tests? We'll probably have to make some adjustment to https://github.com/silverstripe/silverstripe-framework/blob/4/src/Core/Manifest/ManifestFileFinder.php#L213, but that at least gives more flexibility to the community, and we don't end up with another repo like frameworktest.

michalkleiner commented 4 years ago

If there are edge cases that the framework allows, I'd say the tests for those should be in the framework's test suite, otherwise, as @ScopeyNZ says, you may break things you don't know about.

Cheddam commented 4 years ago

I don't like the idea of having a module that the Behat tests rely on, but isn't included in require-dev, as contributors need to be informed of this extra requirement when they cause Behat failures that need to be fixed.

This is a fair point - it should remain possible to run a module's Behat tests locally without manual work (beyond the headache of getting Behat itself working.) Implicitly depending on a module through Travis config would break that assumption.