Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Move test helpers to psammead #80

Merged
merged 10 commits into from
Dec 3, 2018
Merged

Move test helpers to psammead #80

merged 10 commits into from
Dec 3, 2018

Conversation

dr3
Copy link
Contributor

@dr3 dr3 commented Nov 29, 2018

Test helpers are required for #79 so having them as a package is useful IMO

  • [ ] Tests added for new features
  • [ ] Test engineer approval

@dr3 dr3 self-assigned this Nov 30, 2018
@dr3 dr3 added the alpha-2 label Nov 30, 2018
Copy link
Contributor

@sareh sareh left a comment

Choose a reason for hiding this comment

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

Looks great. Just some points to add in the Readme to make it clear how to use the component.

packages/utilities/psammead-test-helpers/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@sareh sareh left a comment

Choose a reason for hiding this comment

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

Looks great. 👍

Copy link
Contributor

@ChrisBAshton ChrisBAshton left a comment

Choose a reason for hiding this comment

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

Hmmm tbh I don't see much value in this being a package, rather than just living at the root level of the repo - it's not a component, and it's not a utility that will be consumed by components. Not something we'll need versioning for.

e.g. will we be using this code directly to run tests, or will we have an npm install @bbc/psammead-test-helpers in our CI? Is that confusing?

I don't feel too strongly about it, but want to know that we've considered it first. 🤔

@sareh
Copy link
Contributor

sareh commented Dec 3, 2018

@ChrisBAshton We'll need these test helpers in both Psammead and in Simorgh, since we'll still have Containers in Psammead that need snapshot tests. For that reason, it makes sense to have this as a single package that can be used by both repositories.

In future, other teams with their own component libraries could use the package as a quick way to get started with their tests.

Copy link
Contributor

@ChrisBAshton ChrisBAshton left a comment

Choose a reason for hiding this comment

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

Fair 👍

@dr3 dr3 merged commit df3b8f3 into latest Dec 3, 2018
@ChrisBAshton ChrisBAshton deleted the psammead-test-helpers branch December 3, 2018 10:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants