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

refactor: sdk wallet integration into components #2532

Merged
merged 78 commits into from
Aug 4, 2020

Conversation

alexbarnsley
Copy link
Member

@alexbarnsley alexbarnsley commented Jul 27, 2020

Summary

Changes components that branch from the Dashboard Wallet list component. Took a while with a lot of changes so I think it may be messy in areas

Checklist

  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@ghost ghost added Complexity: Undetermined Needs specialized, in-depth review. Type: Refactor The pull request improves or enhances an existing implementation. labels Jul 27, 2020
@alexbarnsley
Copy link
Member Author

alexbarnsley commented Jul 31, 2020

This PR is pretty much ready functionality-wise (waiting on #2570 to properly test though).

What isn't ready is the memory limit where tests are now failing, I think because of the extra tests/additional functionality. I'm kind of stuck now, probably because of how large I made this PR

I was debugging and noticed that if I invalidate the jest.setup.js file (e.g. import a package that doesn't exist), all tests fail to run as expected, but I still get the Out of Memory error at the end (see image below). I didn't expect this as none of the tests are running, only the imports.

image

jest.setup.js:

import { env } from "./src/utils/testing-library";
import invalidPackage from "invalid-package";

To run:

yarn test . --runInBand

I think it could be this line in utils/testing-library since removing it stops the out of memory, but it could just be a coincidence.

@clucasalcantara @luciorubeens @goga-m @brenopolanski @dated if anyone has any ideas, that would be great! I think I'm just clutching at straws now.

TL;DR I'm stuck

@alexbarnsley alexbarnsley mentioned this pull request Aug 2, 2020
3 tasks
@alexbarnsley
Copy link
Member Author

@faustbrian I'm aware of the conflicts - sorting them along with fixing tests based on changes that were made before pushing.

@goga-m
Copy link
Contributor

goga-m commented Aug 3, 2020

@alexbarnsley tried the case with invalid package in 3.0-react as well (that has the memory fix) and reproduced Out of Memory error. I assume that this would be caused as a side-effect from --runInBand and gc not running properly on thrown errors. Running it without --runInBand seems to go through the errors without hitting memory limit.

@codecov
Copy link

codecov bot commented Aug 3, 2020

Codecov Report

❗ No coverage uploaded for pull request base (feat/dashboard-wallet-list@616ce08). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@                      Coverage Diff                       @@
##             feat/dashboard-wallet-list     #2532   +/-   ##
==============================================================
  Coverage                              ?   100.00%           
==============================================================
  Files                                 ?       253           
  Lines                                 ?      2896           
  Branches                              ?       517           
==============================================================
  Hits                                  ?      2896           
  Misses                                ?         0           
  Partials                              ?         0           
Flag Coverage Δ
#unit 100.00% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 616ce08...0372e06. Read the comment docs.

@faustbrian faustbrian merged commit 8a51dbf into feat/dashboard-wallet-list Aug 4, 2020
@ghost ghost deleted the refactor/pass-in-sdk-wallet branch August 4, 2020 01:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Complexity: Undetermined Needs specialized, in-depth review. Type: Refactor The pull request improves or enhances an existing implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants