Code submission for FLIP fest 'JS Testing Feature: Multiple return values from interaction 30' #57

hichana · 2021-10-29T07:09:24Z

Closes N/A

Description

Our code submission for FLIP fest issue #30 -- changes include:

a tuple as the return type from sendTransaction and executeTransaction, which also provides error messaging to other functions in the library that implement them.
refactored library internal methods to facilitate the new return types, including jest helper functions
refactored tests
updated all relevant documentation, including CHANGLOG.md

For contributor use:

[x ] Targeted PR against master branch
[x ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work
[x ] Code follows the standards mentioned here
[x ] Updated relevant documentation
[x ] Re-reviewed Files changed in the Github PR explorer
[n/a ] Added appropriate labels

MaxStalker

Looks good! :)
I've left some comments here and there.

dev-test/utilities.test.js

docs/api.md

package.json

src/account.js

src/contract.js

hichana · 2021-11-03T06:28:13Z

Hi @MaxStalker -- thanks for the review. All the changes have been made, but happy to change further if required. Thank you

dev-test/metadata.test.js

dev-test/utilities.test.js

package.json

src/account.js

…o resolve types

…correctly, allowing flow-cadut to resolve

…nning `npm install`

…n package.json

…to update package-lock

hichana · 2021-11-04T03:43:46Z

Hi @MaxStalker , I made all suggested changes for the flow-js-testing flip #30. I tried to be as thorough as possible, like making sure all errors were handled correctly and args passed in order to let flow-cadut handle them instead of the flow types lib. All tests pass and everything seems to work well. Latest branch/commit HERE.

I also resolved the merge conflicts within this github PR, and then pulled those changes back to my local master branch. Unfortunately tests are not passing now. I can run my latest work in the branch I pasted above and everything passes, but then running in the now merged version things fail. Please let me know if we can chat to resolve at your earliest convenience. It seems like our work is not aligning with your latest release, which seems to have happened after our initial fork of the library.

J00lz/cleanup

add fix to generated code to mitigate error running tests

MaxStalker

Looks great! 🤩
I would suggest to run npm run prettify to align the code.

hichana · 2021-11-04T23:47:53Z

Ran prettify @MaxStalker!

MaxStalker

Some more small tweaks, お願いします 🙇‍♂️

src/interaction.js

hichana · 2021-11-05T21:08:43Z

大丈夫！Thanks for catching that limit disappeared @MaxStalker . Re-added and committed to this PR branch.

MaxStalker

お疲れ様です! 🙇

* Code submission for FLIP fest 'JS Testing Feature: Multiple return values from interaction 30' (#57) * squash merge of TeamExponential work for flip 30 * add fixes to final submission requested by Max * add remaining fixes for Max feedback * get basic fixes done requested by Max * update all usage of args for sendTransaction to allow flow-cadut to to resolve types * update all points where executeScript is used to make sure args used correctly, allowing flow-cadut to resolve * remove fcl/types * revert package-lock.json to master plus package.json changes after running `npm install` * revert ports in tests back to 8080, remove testMatch for 'dev-test' in package.json * squash merge of latest changes per Max feedback * make package.json flow-cadut match recent release from Max * revert package-lock.json to master verion * update version in package.json to match Max release, run npm install to update package-lock * ran npm install * squash merge latest updates/fixes * replace package-lock.json * revert package-lock.json * remove try catch from tests * remove onflow/types * remove space * prettify * update cadut to release version * add fix to generated code to mitigate error running tests * ran prettify * re-add 'limit' prop and usage in interaction.js Co-authored-by: justjoolz <[email protected]> Co-authored-by: Maksim Daunarovich <[email protected]> * Bump version to next major release * Add release notes with breaking changes. Co-authored-by: Matt Chana <[email protected]> Co-authored-by: justjoolz <[email protected]>

squash merge of TeamExponential work for flip 30

545b622

hichana requested a review from MaxStalker as a code owner October 29, 2021 07:09

hichana mentioned this pull request Oct 29, 2021

JS Testing Feature: Multiple return values from interaction - Milestone 1 onflow/flip-fest#91

Merged

MaxStalker suggested changes Nov 1, 2021

View reviewed changes

hichana added 2 commits November 2, 2021 17:08

add fixes to final submission requested by Max

61d54c8

add remaining fixes for Max feedback

8fd06a7

MaxStalker suggested changes Nov 3, 2021

View reviewed changes

dev-test/metadata.test.js Outdated Show resolved Hide resolved

dev-test/utilities.test.js Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

src/account.js Outdated Show resolved Hide resolved

hichana and others added 17 commits November 4, 2021 09:48

get basic fixes done requested by Max

04f36b4

update all usage of args for sendTransaction to allow flow-cadut to t…

8287a5b

…o resolve types

update all points where executeScript is used to make sure args used …

5ad827b

…correctly, allowing flow-cadut to resolve

remove fcl/types

c30814d

revert package-lock.json to master plus package.json changes after ru…

435de04

…nning `npm install`

revert ports in tests back to 8080, remove testMatch for 'dev-test' i…

85aadd2

…n package.json

squash merge of latest changes per Max feedback

adac868

make package.json flow-cadut match recent release from Max

dd9201c

revert package-lock.json to master verion

3bf8d30

update version in package.json to match Max release, run npm install …

a491b67

…to update package-lock

ran npm install

0a72976

squash merge latest updates/fixes

844e893

replace package-lock.json

70b11b6

resolve package-lock.json conflicts

f5e1e3c

revert package-lock.json

1081520

resolve package-lock.json conflicts

e6d5378

Merge branch 'master' into master

b384e7f

justjoolz added 4 commits November 4, 2021 17:31

remove try catch from tests

8c5f2aa

remove onflow/types

b956011

remove space

dcabadd

prettify

89eea14

justjoolz and others added 5 commits November 4, 2021 17:35

update cadut to release version

ae34bc9

Merge branch 'master' into j00lz/cleanup

1fecd04

Merge pull request #1 from justjoolz/j00lz/cleanup

de0bd7c

J00lz/cleanup

add fix to generated code to mitigate error running tests

2ca1d90

Merge pull request #2 from justjoolz/flip-fest-submission-fix

9fc548a

add fix to generated code to mitigate error running tests

kerrywei requested a review from MaxStalker November 4, 2021 17:46

MaxStalker approved these changes Nov 4, 2021

View reviewed changes

ran prettify

2f21850

MaxStalker suggested changes Nov 5, 2021

View reviewed changes

src/interaction.js Outdated Show resolved Hide resolved

src/interaction.js Outdated Show resolved Hide resolved

src/interaction.js Outdated Show resolved Hide resolved

src/interaction.js Outdated Show resolved Hide resolved

re-add 'limit' prop and usage in interaction.js

632b5cb

MaxStalker approved these changes Nov 5, 2021

View reviewed changes

MaxStalker changed the base branch from master to max/fli-merge December 8, 2021 16:08

Merge branch 'max/fli-merge' into master

8cb27a3

MaxStalker merged commit b7e98aa into onflow:max/fli-merge Dec 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code submission for FLIP fest 'JS Testing Feature: Multiple return values from interaction 30' #57

Code submission for FLIP fest 'JS Testing Feature: Multiple return values from interaction 30' #57

hichana commented Oct 29, 2021

MaxStalker left a comment

hichana commented Nov 3, 2021

hichana commented Nov 4, 2021 •

edited

Loading

MaxStalker left a comment

hichana commented Nov 4, 2021

MaxStalker left a comment

hichana commented Nov 5, 2021

MaxStalker left a comment

Code submission for FLIP fest 'JS Testing Feature: Multiple return values from interaction 30' #57

Code submission for FLIP fest 'JS Testing Feature: Multiple return values from interaction 30' #57

Conversation

hichana commented Oct 29, 2021

Description

MaxStalker left a comment

Choose a reason for hiding this comment

hichana commented Nov 3, 2021

hichana commented Nov 4, 2021 • edited Loading

MaxStalker left a comment

Choose a reason for hiding this comment

hichana commented Nov 4, 2021

MaxStalker left a comment

Choose a reason for hiding this comment

hichana commented Nov 5, 2021

MaxStalker left a comment

Choose a reason for hiding this comment

hichana commented Nov 4, 2021 •

edited

Loading