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

New module for testing edgecases #9712

Open
brynwhyman opened this issue Sep 28, 2020 · 5 comments
Open

New module for testing edgecases #9712

brynwhyman opened this issue Sep 28, 2020 · 5 comments

Comments

@brynwhyman
Copy link
Contributor

brynwhyman commented Sep 28, 2020

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
Copy link
Contributor Author

An example where this module would be used: silverstripe/silverstripe-admin#1122

@sminnee
Copy link
Member

sminnee commented Sep 28, 2020

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
Copy link
Contributor

ScopeyNZ commented Sep 28, 2020

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
Copy link
Contributor

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
Copy link
Member

Cheddam commented Sep 29, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants