Skip to content

Comments

Fixes task download button#2275

Merged
gingi merged 2 commits intomasterfrom
bugfix/task-download-button
Feb 4, 2021
Merged

Fixes task download button#2275
gingi merged 2 commits intomasterfrom
bugfix/task-download-button

Conversation

@gingi
Copy link
Member

@gingi gingi commented Feb 4, 2021

No description provided.

@gingi gingi added this to the 2.9.0 milestone Feb 4, 2021
@gingi gingi enabled auto-merge February 4, 2021 00:16
@gingi gingi linked an issue Feb 4, 2021 that may be closed by this pull request
Copy link
Member

@dpwatrous dpwatrous left a comment

Choose a reason for hiding this comment

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

Good catch!

Not a blocker, but mind changing _saveFile() to take a string instead of any? If it had been typed we would have caught this on upgrading Electron.

@dpwatrous
Copy link
Member

Looks like there's also a unit testing issue (the call is probably just being mocked incorrectly now)

@gingi gingi force-pushed the bugfix/task-download-button branch from be65b9f to 47d8a54 Compare February 4, 2021 16:01
@gingi
Copy link
Member Author

gingi commented Feb 4, 2021

Looks like there's also a unit testing issue (the call is probably just being mocked incorrectly now)

Fixed. Had to make several adjustments for promise-based testing.

@dpwatrous dpwatrous self-requested a review February 4, 2021 16:05
@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #2275 (862a6a1) into master (37b5261) will decrease coverage by 0.03%.
The diff coverage is 37.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2275      +/-   ##
==========================================
- Coverage   65.23%   65.19%   -0.04%     
==========================================
  Files         903      903              
  Lines       25693    25707      +14     
  Branches     5102     5102              
==========================================
- Hits        16761    16760       -1     
- Misses       8932     8947      +15     
Impacted Files Coverage Δ
.../services/network/network-configuration.service.ts 12.50% <0.00%> (-2.21%) ⬇️
src/app/utils/pool-utils.ts 82.47% <ø> (ø)
...l/action/add/pool-create-basic-dialog.component.ts 9.80% <5.00%> (-0.43%) ⬇️
...-flask/electron/testing/electron-testing.module.ts 75.00% <100.00%> (+2.27%) ⬆️
...file-viewer-header/file-viewer-header.component.ts 87.03% <100.00%> (+0.24%) ⬆️
...r/rpc-server-status/rpc-server-status.component.ts 76.66% <100.00%> (ø)
...network-picker/virtual-network-picker.component.ts 83.75% <100.00%> (ø)
src/app/models/pool-os-skus.ts 100.00% <100.00%> (+11.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d965d4e...81f600e. Read the comment docs.

@gingi gingi force-pushed the bugfix/task-download-button branch from 47d8a54 to ad51945 Compare February 4, 2021 17:18
Electron's `dialog.showSaveDialog` has changed at some point to return a `Promise` rather than a string value. Adjusted to the new API.
@gingi gingi force-pushed the bugfix/task-download-button branch from ad51945 to 81f600e Compare February 4, 2021 18:07
@gingi gingi merged commit 729bda7 into master Feb 4, 2021
@gingi gingi deleted the bugfix/task-download-button branch February 4, 2021 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot download task output files

2 participants