EjectButton component test

[AWolf81]

Related Issue:

309

Summary: Slightly modified EjectButton with two named exports dialogOptions and dialogCallback so they can be used in the test.

Tests:

• Rendering (running and not running) with snapshot tests
• Test dialog
  • Dialog displayed
  • Confirm
  • Dismiss

[codecov[bot]]

Codecov Report

Merging #348 into master will increase coverage by 0.44%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #348      +/-   ##
==========================================
+ Coverage   24.91%   25.35%   +0.44%     
==========================================
  Files         152      152              
  Lines        3625     3613      -12     
  Branches      388      388              
==========================================
+ Hits          903      916      +13     
+ Misses       2450     2426      -24     
+ Partials      272      271       -1

Impacted Files	Coverage Δ
src/components/EjectButton/EjectButton.js	100% <100%> (+100%)	:arrow_up:
...omponents/BigClickableButton/BigClickableButton.js	20% <0%> (+20%)	:arrow_up:

[superhawk610]

Hey @AWolf81, long time no see 😃

I don't feel comfortable Approving this PR since I've been away from the project for a while, but it looks good to me 👍

My only feedback is that I always avoid func.bind(context) at all costs, but I believe this is one of those rare cases where it's required. The tests are thorough as well.

[AWolf81]

@superhawk610 hey, no problem.

I also always try to avoid bind as arrow functions are usually more readable but here I think it's OK as it's shorter than response => dialogCallback(response). But binding should be also OK here. I just replaced the mentioned arrow function as it was marked as uncovered in the coverage report.

I first tried to mock the dialog.showMessagebox but I wasn't sure how I could test and mock the callback.

[melanieseltzer]

Do you think we should convert the other dialogs to this format without the handleClick? e.g. in DeleteDependencyButton.js

[AWolf81]

@melanieseltzer yes, the test for that component will be similar to the EjectButton. I think it's also OK to inline the handleClick just add the logic before calling () => dependencyStatus === 'idle' && dialog.showMessageBox(options, callback.bind(this)) and in the callback destructure the required props.

If you like you can create a DeleteDependencyButton.test.js and export the dialogOptions & callback.

[melanieseltzer]

👍 I'll take a stab at converting the rest of the dialogs when I have some free time, I think it'll be good to have everything uniform.