Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: use "given" keyword in branching trees #656

Merged
merged 9 commits into from
Aug 23, 2023
Merged

test: use "given" keyword in branching trees #656

merged 9 commits into from
Aug 23, 2023

Conversation

PaulRBerg
Copy link
Member

@PaulRBerg PaulRBerg commented Aug 21, 2023 •
edited
Loading

@smol-ninja @PaulRBerg
refactor: using Given keyword in branching Trees (#641)
cf74641 
* Using Given keyword on the branching trees

(cherry picked from commit d365705)

* test: use "it" for test nodes

---------

Co-authored-by: Paul Razvan Berg <[email protected]>

test: rename to "givenCallerAdmin"

test: use given keyword in the integration branching trees

test: use given keyword for modifiers for fuzz tests

test: use given keyword in test contracts and brancing trees

test: refactor test function names to use Given and When

test: refactor test function names to use Given and When

refactor: reverting in dev comments

refactor: rename test function names to cleaner form

test: polish branching trees

test: improve "mapSymbol" tree
test: improve "safeAssetDecimals" tree
test: improve "safeAssetSymbol" tree
@PaulRBerg PaulRBerg changed the title refactor: use "given" keyword in branching trees test: use "given" keyword in branching trees Aug 21, 2023
@PaulRBerg
Copy link
Member Author

@andreivladbrg if you could have a quick look that would be helpful.

@PaulRBerg PaulRBerg mentioned this pull request Aug 21, 2023
Merged
andreivladbrg
andreivladbrg requested changes Aug 22, 2023
View reviewed changes
Copy link
Member

@andreivladbrg andreivladbrg left a comment

Choose a reason for hiding this comment

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

Missed tree:

https://github.com/sablier-labs/v2-core/blob/test/improve-tree-wording/test/integration/concrete/lockup/cancel-multiple/cancelMultiple.tree

Should we use here "given?

v2-core/test/integration/concrete/lockup/cancel/cancel.tree

Line 37 in aedff61

│ ├── when the recipient does not implement the hook

v2-core/test/integration/concrete/lockup/cancel/cancel.tree

Line 64 in aedff61

├── when the sender is not a contract

v2-core/test/integration/concrete/lockup/cancel/cancel.tree

Line 68 in aedff61

├── when the sender does not implement the hook

test/integration/concrete/lockup-dynamic/create-with-milestones/createWithMilestones.t.sol Show resolved Hide resolved
test/integration/concrete/lockup-linear/create-with-range/createWithRange.t.sol Show resolved Hide resolved
test/integration/concrete/lockup-dynamic/streamed-amount-of/streamedAmountOf.tree Show resolved Hide resolved
test/integration/concrete/lockup/cancel/cancel.tree Outdated Show resolved Hide resolved
test/integration/concrete/lockup/cancel/cancel.t.sol Outdated Show resolved Hide resolved
test/integration/concrete/nft-descriptor/map-symbol/mapSymbol.tree Show resolved Hide resolved
test/integration/concrete/nft-descriptor/safe-asset-decimals/safeAssetDecimals.tree Outdated Show resolved Hide resolved
test/integration/concrete/nft-descriptor/safe-asset-symbol/safeAssetSymbol.tree Outdated Show resolved Hide resolved
test/integration/fuzz/lockup-dynamic/streamedAmountOf.t.sol Outdated Show resolved Hide resolved
test/integration/fuzz/lockup/cancel.t.sol Outdated Show resolved Hide resolved
@andreivladbrg andreivladbrg mentioned this pull request Aug 22, 2023
Merged
@PaulRBerg
Copy link
Member Author

Thanks for your review, @andreivladbrg.

Should we use here "given?

Yes, we should use "given" for the recipient branches in the cancel.t.sol tests.

@PaulRBerg
Copy link
Member Author

@andreivladbrg, can you approve the PR?

@PaulRBerg PaulRBerg merged commit 925630d into main Aug 23, 2023
9 checks passed
@PaulRBerg PaulRBerg deleted the test/improve-tree-wording branch August 23, 2023 17:29
@alexfertel alexfertel mentioned this pull request Aug 24, 2023
Closed
andreivladbrg added a commit that referenced this pull request Oct 20, 2023
@scorpion9979 @PaulRBerg
@andreivladbrg @antoncoding @allwin199 @PraneshASP @smol-ninja @alexfertel @Lumyo
Merge "main" branch into "ci/slither" (#713)
d00fd89 
* docs: delete wikis

* test: polish precompiles tests

build: add branch for solady in ".gitmodules"

* docs: polish deployments section

* docs: polish security program

* docs: polish license section

* chore: remove Codecov token

* refactor: update gas snapshot

* build: change version to 1.0.0-rc.0

chore: update "repository" in "package.json"

* refactor: stringify create2 salts

* refactor: rename deployer to broadcaster

feat: use $ETH_FROM as broadcaster

* chore: fix rpc url for gnosis chain

* docs: refine license section

* test: update createSelectFork

* chore: use solhint-community

build: add "solhint-community" node.js dep
build: remove "solhint" node.js

* build: bump solady

* build: release v1.0.0

* test: remove duplicate bounds

* docs: mention audits in the bug bounty

* chore: say token streaming

* test: rename "basic" to "concrete"

* test: order imports correctly

* perf: use vars.sablierAddress

Closes #607

* perf: define "vars.asset" as "address"

Closes #608

* fix: data URI scheme

test: update hard-coded values
test: use LibString to remove data URI prefix

Closes #616

* test: update Precompiles bytecodes

* docs: roll v1.0.1

docs: add CHANGELOG

* docs: update recommended install branch

* chore: add missing modifier in test_SetFlashFee test

* Add Contribution info in README.md

* test: convert 'flashFee' integration tests to unit tests

* test: convert 'setFlashFee' integration tests to unit tests

* test: add 'Comptroller' setup for concrete unit test

* test: update 'flashFee' unit-test to inherit from Comptroller

* test: update 'setFlashFee' unit-test to inherit from Comptroller

* style: run prettier on README

* test: deploy correctly precompiled comptroller

* feat: deploy core 2

* test: change default profile value

* Fix error when importing @prb/math packages when installed via npm (#648)

* fix: address #646

* chore: update remapping in slither config

---------

Co-authored-by: Paul Razvan Berg <[email protected]>

* test: fix string parameter

* docs: roll v1.0.2

* docs: update recommended remapping

* docs: add "Architecture" in README

closes #551

* perf: dry "isCold" and "isWarm"

Closes #610

* docs: fix nitpicks

Closes #611

* test: use "given" keyword in branching trees (#656)

* refactor: using Given keyword in branching Trees (#641)

* Using Given keyword on the branching trees

(cherry picked from commit d365705)

* test: use "it" for test nodes

---------

Co-authored-by: Paul Razvan Berg <[email protected]>

test: rename to "givenCallerAdmin"

test: use given keyword in the integration branching trees

test: use given keyword for modifiers for fuzz tests

test: use given keyword in test contracts and brancing trees

test: refactor test function names to use Given and When

test: refactor test function names to use Given and When

refactor: reverting in dev comments

refactor: rename test function names to cleaner form

test: polish branching trees

test: improve "mapSymbol" tree
test: improve "safeAssetDecimals" tree
test: improve "safeAssetSymbol" tree

* refactor: givenWithdrawableAmountNotZero modifier name

* test: use "when" for time-related branches

* test: address feedback in tree wording branch

* test: polish branching trees

* fix: remove duplicated branch in createWithMilestones.tree

---------

Co-authored-by: Shub <[email protected]>
Co-authored-by: andreivladbrg <[email protected]>
Co-authored-by: Alexander González <[email protected]>

* docs: mention audits

* docs: add Discord badge

* test: update comment

* fix: fix incorrect spec in isStream.tree (#677)

* test: use "given" instead of "when" (#678)

* test: use "given" instead of "when"

* test: correct "given" branches

---------

Co-authored-by: andreivladbrg <[email protected]>

* ci: cache testing contracts (#681)

* Fix Github workflow build cache

* ci: add "out" dir in github cache

* Update .github/workflows/ci.yml

* Update .github/workflows/ci.yml

* Update .github/workflows/ci-deep.yml

---------

Co-authored-by: Lumyo <[email protected]>
Co-authored-by: Paul Razvan Berg <[email protected]>

* feat: add transferrable bool field for stream NFT

* feat: override _beforeTokenTransfer for Stream NFT and test cases

* test: Add test for NFT transfer functionality

* test: add more testing and resolve #669

* perf: dry "_beforeTokenTransfer"

refactor: remove "canStreamTransfer"
refactor: remove double "r" in "transferrable"
refactor: simplify implementation
test: allow non-transferable streams in invariant tests
test: dry tests for "isTransferable"
test: improve function and test names

* test: add a new branch to burn.tree

* test: polish burn tests

* test: update precompiles

* feat: implement _afterTokenTransfer to emit an event

* test: expect MetadataUpdate event to be emitted

* test: update Precompiles bytecode

* test: transferFrom function

* chore: add commented parameters in ERC721 hooks

* docs: improve writing in NatSpec

test: fix typos

* build: upgrade solidity version to 0.8.21 (#688)

* build: upgrade solidity version to 0.8.21

* build: bump the pragma back to >=0.8.19

* build: show unproved and unsupported SMTChecker

* refactor: update gas snapshot

---------

Co-authored-by: Paul Razvan Berg <[email protected]>

* refactor: change the streamed amount to total amount in NFT SVG (#692)

* refactor: change the streamed card from NFT SVG with total

build: update shell scripts accordingly
test: update tests accordingly

* chore: say "total amount" in comments for total card

* refactor: change the "Total" from NFT SVG with "Amount"

build: update shell script accordingly
test: update tests accordingly

* chore: fix typo in shell script

* refactor: dry-fy withdraw function

refactor: dry-fy renounce function

* build: remove OZ from peer dependencies

* docs: roll 1.1.0 (#693)

* docs: roll 1.1.0

* docs: update changelog

---------

Co-authored-by: Paul Razvan Berg <[email protected]>

* chore: fix typo in DataTypes

* docs: correct github hyperlink

docs: order change logs chronologically

* docs: update changelog

* build: switch to "solhint"

chore: disable Solhint rules

* refactor: capitalize immutable variables  (#700)

* refactor: capitalize immutable variables in NoDelegateCall

build: remove "immutable-vars-naming" from solhint file

* test: make asset and holder immutable in fork tests

test: capitalize constans in fork tests

* docs: polish background description

* chore: remove "cbor_metadata"

* perf: optimize withdraw function

* Remove the ability to cancel for recipients (#710)

* feat: remove ability to cancel for recipient

feat: remove sender's hook
test: update tests accordingly

* feat: emit asset in cancel and withdraw events

refactor: remove recipient from cancel event
test: update tests accordingly
test: update precompiles bytecode

* chore: update gas snapshot

* test: update Precompiles bytecode

* docs: include recipient disable to cancel in changelog

* refactor: add "recipient" in cancel event

docs: refine explanatory comments
refactor: reorder parameters in events

* test: update Precompiles bytecode

* chore: add explantory comment

---------

Co-authored-by: Paul Razvan Berg <[email protected]>

* test: add DeployOptimized utils contracts (#712)

* test: add DeployOptimized utils contracts

test: move and rename functions from Base_Test

* docs: fix typo in comments

* test: rename deploy optimized functions

---------

Co-authored-by: Paul Razvan Berg <[email protected]>

* style: fix Prettier formatting issue

---------

Co-authored-by: Paul Razvan Berg <[email protected]>
Co-authored-by: andreivladbrg <[email protected]>
Co-authored-by: Anton Cheng <[email protected]>
Co-authored-by: Prince Allwin <[email protected]>
Co-authored-by: PraneshASP <[email protected]>
Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>
Co-authored-by: Shub <[email protected]>
Co-authored-by: Alexander González <[email protected]>
Co-authored-by: Lumyo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreivladbrg andreivladbrg andreivladbrg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Separate current state from current action in Branching Trees
4 participants
@PaulRBerg @andreivladbrg @smol-ninja @alexfertel