Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Component unit tests enzyme #340

Merged
merged 8 commits into from
Jan 6, 2019
Merged

Component unit tests enzyme #340

merged 8 commits into from
Jan 6, 2019

Conversation

AWolf81
Copy link
Collaborator

@AWolf81 AWolf81 commented Dec 13, 2018

Related Issue:
#309

Summary:

  • Added Enzyme setup
  • Removed flow typing from tests
  • Added first tests
    • Progressbar
    • Sidebar

Tests

  • Progressbar component
  • Sidebar component

@codecov
Copy link

codecov bot commented Jan 3, 2019

Codecov Report

Merging #340 into master will increase coverage by 4.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #340      +/-   ##
==========================================
+ Coverage   20.73%   24.86%   +4.12%     
==========================================
  Files         248      152      -96     
  Lines        3945     3628     -317     
  Branches      391      389       -2     
==========================================
+ Hits          818      902      +84     
+ Misses       2846     2452     -394     
+ Partials      281      274       -7
Impacted Files Coverage Δ
...nents/ProjectIconSelection/ProjectIconSelection.js 0% <ø> (ø) ⬆️
src/sagas/task.saga.js 64.56% <ø> (ø) ⬆️
src/components/Sidebar/Sidebar.js 75% <ø> (+75%) ⬆️
src/components/ProgressBar/ProgressBar.js 100% <100%> (+100%) ⬆️
src/components/AddDependencyInitialScreen/index.js
src/components/Toggle/Toggle.stories.js
src/components/AddDependencyModal/index.js
src/components/TwoPaneModal/index.js
src/components/SettingsButton/index.js
src/components/Toggle/index.js
... and 99 more

@AWolf81 AWolf81 changed the title [WIP] Component unit tests enzyme Component unit tests enzyme Jan 3, 2019
@AWolf81
Copy link
Collaborator Author

AWolf81 commented Jan 3, 2019

@idoberko2 would be great if you could review the setup and first tests? If there is nothing against using Enzyme we can merge this to have the setup ready and we can continue to add more tests afterwards.

Copy link
Collaborator

@melanieseltzer melanieseltzer left a comment

Choose a reason for hiding this comment

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

LGTM! Just a question about a file I wasn't sure about.

Also since you were adding /* eslint-disable flowtype/require-valid-file-annotation */ to the tests missing it, do you mind doing it to WhimsicalInstaller.helpers.test.js and Initialization.helpers.test.js? Just found out they are also not disabled and we can just do it in this branch.

@@ -0,0 +1,19 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not familiar with what we're having in the repo for VSCode settings, was this supposed to be committed? If so what does it do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd like to add the launch settings so it's possible to use the integrated debugger during testing - so you can add breakpoints etc. Just open the debugger pane (striked-through bug icon, see screenshot below) select the configuration & click play. (No config in the screenshot)
screenshot_debugger_pane

OK, I'll check the other test files and also disable flowjs there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I've fixed all linting errors. Also added two unrelated flow linting errors.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah that's cool 👍

@idoberko2
Copy link
Collaborator

@AWolf81, how do you want to proceed with this one?
Should I just start covering components with tests or you intend to write some tests on your side (I don't want us to step on each other's tows)?
Should I branch out of this one (in case you're about to merge it)?

@AWolf81
Copy link
Collaborator Author

AWolf81 commented Jan 6, 2019

@idoberko2 if the setup is OK in this PR, we can merge it to master. Can we merge this?

Then you can branch from master and we can track progress on issue #309. Just add the components you'd like to work-on as a comment and I'll add mine (if I'm adding some tests - no progress on my side at the moment).

@idoberko2
Copy link
Collaborator

@AWolf81, the setup looks good and I can confirm it runs successfully on my environment both via the CLI and the VSCode integration.

@AWolf81 AWolf81 merged commit 3f6b899 into master Jan 6, 2019
@AWolf81 AWolf81 deleted the component-unit-tests-enzyme branch January 6, 2019 12:45
@AWolf81 AWolf81 mentioned this pull request Jan 6, 2019
14 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.

3 participants