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

POC: Increase utilities test coverage #224

Closed
wants to merge 39 commits into from

Conversation

pjlee11
Copy link
Contributor

@pjlee11 pjlee11 commented Dec 17, 2018

Resolves #156

  • Adds a test helper method that takes an actual import and validates it has all the exported values it is expected to have. This can then be used to simplifying testing the utility packages exports

  • Has error handling logic for both new files (EG: font.js) and for exported files within those files when:

    1. The actual import has additional exports when compared to the expected values. This is needed so that we do not forget to change the expectations when adding a new export.
    2. The expected value has additional exports as compared to the actual exports. This is needed in the case of a breaking change when an export is to be removed. This is very useful at catching regressions in the utility packages.
  • I have assigned myself to this PR and the corresponding issues

  • Tests added for new features

  • Test engineer approval

To get this work merged in I'll have to split out the changes in psammead-test-helpers and then bump the version and publish to npm before then using the new test helper version in the other packages.

That said, this PR can be tested locally by checking out this branch, then copy and paste the following command, which locally links all the necessary packages that once split out will in the future depend on psammead-test-helpers:

cd packages/utilities/gel-foundations &&
  npm install && npm link ../psammead-test-helpers &&
  cd ../psammead-assets &&
  npm install && npm link ../psammead-test-helpers &&
  cd ../psammead-styles &&
  npm install && npm link ../psammead-test-helpers &&
  cd ../../../ &&
  npm run build && npm run test

Finally, see the test coverage for the whole repo increase from something in the 50's% to over 89% 🎉

@pjlee11 pjlee11 self-assigned this Dec 17, 2018
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.

Nice test approach! Looks good. Just fix the typos 👍

packages/utilities/gel-constants/index.test.jsx Outdated Show resolved Hide resolved
packages/utilities/gel-constants/index.test.jsx Outdated Show resolved Hide resolved
packages/utilities/psammead-test-helpers/src/index.js Outdated Show resolved Hide resolved

describe('Psammead test helpers', () => {
it('should test all the utility exports exist and are the correct type', () => {
testHelpers.testUtilityPackages(
Copy link
Contributor Author

@pjlee11 pjlee11 Dec 17, 2018

Choose a reason for hiding this comment

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

Using one of the test helper functions to test the test helper utility #meta

@pjlee11
Copy link
Contributor Author

pjlee11 commented Dec 17, 2018

This will fail Jenkins and will need psammead-test-helpers merged and published separately from the ahead of the utilities. This PR is to prove that the combined changes have the overall effect of increasing the test coverage in a DRY way

@pjlee11
Copy link
Contributor Author

pjlee11 commented Dec 18, 2018

image

bcmn
bcmn previously approved these changes Dec 18, 2018
@pjlee11 pjlee11 dismissed stale reviews from bcmn via 82d010d January 4, 2019 10:36
ChrisBAshton
ChrisBAshton previously approved these changes Jan 4, 2019
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.

Nice one 👍

bcmn
bcmn previously approved these changes Jan 4, 2019
@pjlee11 pjlee11 dismissed stale reviews from bcmn and ChrisBAshton via 3e3366c January 4, 2019 11:44
@pjlee11 pjlee11 force-pushed the increase-utilities-test-coverage branch from a0445b6 to 3e3366c Compare January 4, 2019 11:44
@pjlee11 pjlee11 changed the title Increase utilities test coverage POC: Increase utilities test coverage Jan 4, 2019
@pjlee11
Copy link
Contributor Author

pjlee11 commented Jan 4, 2019

Due to 3 out of the 4 packages being dependant on changes to the test helper package I'm going to split this PR in two. Therefore closing this PR

@pjlee11 pjlee11 closed this Jan 4, 2019
pjlee11 pushed a commit that referenced this pull request Jan 4, 2019
Cherry pick changes to `psammead-test-helpers` from the POC PR - #224
@ChrisBAshton
Copy link
Contributor

PR 1: #258

@ChrisBAshton ChrisBAshton deleted the increase-utilities-test-coverage branch January 4, 2019 14:58
@pjlee11
Copy link
Contributor Author

pjlee11 commented Jan 4, 2019

PR 2: #259

@thekp thekp mentioned this pull request Jul 12, 2019
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants