Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

5427 contracts unit tests #5465

Merged
jdevcs merged 9 commits into from
Sep 30, 2022
Merged

5427 contracts unit tests #5465

jdevcs merged 9 commits into from
Sep 30, 2022

Conversation

@jdevcs
Copy link
Contributor

@jdevcs jdevcs commented Sep 22, 2022

Description

#5427

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist for 1.x:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run dtslint with success and extended the tests and types if necessary.
  • I ran npm run test:cov and my test cases cover all the lines and branches of the added code.
  • I ran npm run build with success.
  • I have tested the built dist/web3.min.js in a browser.
  • I have tested my code on the live network.
  • I have checked the Deploy Preview and it looks correct.
  • I have updated the CHANGELOG.md file in the root folder.

Checklist for 4.x:

  • I have selected the correct 4.x base branch.
  • Within the description, the feature or issue is discussed in detail or an issue is linked.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added any required tests for the changes I made
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged and published in downstream modules.
  • I ran yarn successfully
  • I ran yarn lint successfully
  • I ran yarn build:web successfully
  • I ran yarn test:unit successfully
  • I ran yarn test:integration successfully
  • I ran compile:contracts successfully
  • I have tested my code.
  • I have updated the corresponding CHANGELOG.md file in the packages I have edited.

@jdevcs jdevcs added the 4.x 4.0 related label Sep 22, 2022
@jdevcs jdevcs self-assigned this Sep 22, 2022
@jdevcs jdevcs marked this pull request as ready for review September 26, 2022 16:13
luu-alex
luu-alex reviewed Sep 27, 2022
View reviewed changes
packages/web3-eth-contract/test/unit/contract.test.ts Outdated
};
expect(contract.defaultCommon).toBeUndefined();
contract.defaultCommon = defaultCommon;
// expect(contract.defaultHardfork).toStrictEqual(defaultCommon); // TODO check if is being fixed in 5419
Copy link
Contributor

@luu-alex luu-alex Sep 27, 2022

Choose a reason for hiding this comment

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

Suggested change
// expect(contract.defaultHardfork).toStrictEqual(defaultCommon); // TODO check if is being fixed in 5419
// expect(contract.defaultCommon).toStrictEqual(defaultCommon); // TODO check if is being fixed in 5419

Did u mean defaultCommon instead of defaultHardFork?

Copy link
Contributor Author

@jdevcs jdevcs Sep 27, 2022

Choose a reason for hiding this comment

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

yes. it was commented earlier, as this should not pass:
in earlier line I set HF diff as compared to contract.defaultCommon.hardfork
it should throw error when contract.defaultHardfork != contract.defaultCommon.hardfork we discussed in #5419 ?

Copy link
Contributor

@luu-alex luu-alex Sep 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

https://github.com/web3/web3.js/blob/4.x/packages/web3-eth/src/validation.ts#L161 It will error when theres a validation check against when common, chain and hardfork are all defined

Copy link
Contributor Author

@jdevcs jdevcs Sep 28, 2022

Choose a reason for hiding this comment

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

Thanks, so validation is done at validateTransactionForSigning level.

I think we should have additional feature of doing validation during setting defaults as well, this will help users to identify errors at earlier stage.

Copy link
Contributor

@luu-alex luu-alex Sep 28, 2022

Choose a reason for hiding this comment

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

sounds good, that can be a different feature. pr looks good to me

Muhammad-Altabba
Muhammad-Altabba reviewed Sep 27, 2022
View reviewed changes
Copy link
Contributor

@Muhammad-Altabba Muhammad-Altabba left a comment

Choose a reason for hiding this comment

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

Looks great.
However, I have just 2 questions.

@Muhammad-Altabba Muhammad-Altabba self-requested a review September 27, 2022 15:43
Muhammad-Altabba
Muhammad-Altabba suggested changes Sep 27, 2022
View reviewed changes
Copy link
Contributor

@Muhammad-Altabba Muhammad-Altabba left a comment

Choose a reason for hiding this comment

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

Looks great!
However, there are fixable issues in the pipeline I think.

@jdevcs jdevcs mentioned this pull request Sep 28, 2022
Closed
@jdevcs jdevcs merged commit 903d642 into 4.x Sep 30, 2022
@jdevcs jdevcs deleted the junaid/contractsunitests branch September 30, 2022 10:05
spacesailor24 added a commit that referenced this pull request Oct 5, 2022
@Muhammad-Altabba @nikoulai
@jdevcs @spacesailor24
Fix typing compatibility when linking a contract to a context (#5416)
7acb576 
* fix typing compatibility when linking a contract to a context

* replace any with unknown at a suggested code for Web3Context

* Web3 net system tests (#5457)

* Add tests

* Update geth script

* Fix geth script

* Reset to initial chainId

* Remove uneeded space

* Ganace chainId

* more fixes and sample calls for the plugin draft

* tiny code revert for unused variable

* add comments and replace some `any`s with `unknown`s for plugin related code

* 5427 contracts unit tests (#5465)

* sample abi of a contract

* unit tests contract

* contract events test

* unit test data

* additional tests for more coverage

* default common check

* test contract source and updated abi

* common type fix

* Merge with 5091

Co-authored-by: Nikos Iliakis <[email protected]>
Co-authored-by: Junaid <[email protected]>
Co-authored-by: Wyatt Barnes <[email protected]>
@luu-alex luu-alex mentioned this pull request Oct 10, 2022
Merged
31 tasks
spacesailor24 added a commit that referenced this pull request Oct 25, 2022
@spacesailor24 @jdevcs
@Muhammad-Altabba @nikoulai
Example Plugin Implementation (#5393)
c83a36e 
* Init abstract Web3PluginBase class. Init registerPlugin method

* Update web3.ts to have a named export instead of using default

* Init web3-plugin-example package

* Update CHANGELOG and README

* Update packages/web3-plugin-example/test/unit/plugin.test.ts

* Remove unneeded npm scripts

* Remove webpack config

* Init ExistingPluginNamespaceError

* registerPluging - throw error is namespace not undefined

* Update Web3 to use default and named exports

* Add peerDependencies

* Default API generic to Web3APISpec for Web3PluginBase

* Remove use of ChainlinkPluginAPI. Define return type for getPrice

* Remove no longer needed generic values

* Override link method for plugin

* Remove no longer used import

* Remove ChainlinkPlugin code

* Init custom_rpc_methods plugin example

* Init contract_method_wrappers plugin example

* Update web3 import to remove import ts error

* Fix typing compatibility when linking a contract to a context (#5416)

* fix typing compatibility when linking a contract to a context

* replace any with unknown at a suggested code for Web3Context

* Web3 net system tests (#5457)

* Add tests

* Update geth script

* Fix geth script

* Reset to initial chainId

* Remove uneeded space

* Ganace chainId

* more fixes and sample calls for the plugin draft

* tiny code revert for unused variable

* add comments and replace some `any`s with `unknown`s for plugin related code

* 5427 contracts unit tests (#5465)

* sample abi of a contract

* unit tests contract

* contract events test

* unit test data

* additional tests for more coverage

* default common check

* test contract source and updated abi

* common type fix

* Merge with 5091

Co-authored-by: Nikos Iliakis <[email protected]>
Co-authored-by: Junaid <[email protected]>
Co-authored-by: Wyatt Barnes <[email protected]>

* Fix lint error

* Move web3-plugin-example to tools/. Remove @ts-expect-errors. Move Web3 module redeclarations to test files

* Update CHANGELOGs

* Fix tsc and linter errors

* Update packages/web3-types/CHANGELOG.md

* Add CHANGELOG changes to root CHANGELOG

* Update packages/web3-types/CHANGELOG.md

* Update CHANGELOG.md

Co-authored-by: Muhammad Altabba <[email protected]>

* Update CHANGELOG.md

* Add web3-plugin-example CHANGELOG

* Remove unnecessary web3 imports

* Revert change to Web3 export

* Init web3_export_helper.ts

* Update packages/web3/CHANGELOG.md

Co-authored-by: Muhammad Altabba <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Muhammad Altabba <[email protected]>

* Update comments for mocking in contract_method_wrappers.test.ts

Co-authored-by: Junaid <[email protected]>
Co-authored-by: Muhammad Altabba <[email protected]>
Co-authored-by: Nikos Iliakis <[email protected]>
spacesailor24 added a commit that referenced this pull request Oct 25, 2022
@Muhammad-Altabba @nikoulai
@jdevcs @spacesailor24
Fix typing compatibility when linking a contract to a context (#5416)
bb41fb7 
* fix typing compatibility when linking a contract to a context

* replace any with unknown at a suggested code for Web3Context

* Web3 net system tests (#5457)

* Add tests

* Update geth script

* Fix geth script

* Reset to initial chainId

* Remove uneeded space

* Ganace chainId

* more fixes and sample calls for the plugin draft

* tiny code revert for unused variable

* add comments and replace some `any`s with `unknown`s for plugin related code

* 5427 contracts unit tests (#5465)

* sample abi of a contract

* unit tests contract

* contract events test

* unit test data

* additional tests for more coverage

* default common check

* test contract source and updated abi

* common type fix

* Merge with 5091

Co-authored-by: Nikos Iliakis <[email protected]>
Co-authored-by: Junaid <[email protected]>
Co-authored-by: Wyatt Barnes <[email protected]>
luu-alex pushed a commit that referenced this pull request Oct 29, 2022
@spacesailor24 @Muhammad-Altabba
@nikoulai @jdevcs
Init Plugin Docs (#5558)
173bca5 
* Update web3.ts to have a named export instead of using default

* Init web3-plugin-example package

* Update CHANGELOG and README

* Update packages/web3-plugin-example/test/unit/plugin.test.ts

* Remove unneeded npm scripts

* Remove webpack config

* Update Web3 to use default and named exports

* Add peerDependencies

* Remove use of ChainlinkPluginAPI. Define return type for getPrice

* Override link method for plugin

* Remove ChainlinkPlugin code

* Init custom_rpc_methods plugin example

* Init contract_method_wrappers plugin example

* Update web3 import to remove import ts error

* Fix typing compatibility when linking a contract to a context (#5416)

* fix typing compatibility when linking a contract to a context

* replace any with unknown at a suggested code for Web3Context

* Web3 net system tests (#5457)

* Add tests

* Update geth script

* Fix geth script

* Reset to initial chainId

* Remove uneeded space

* Ganace chainId

* more fixes and sample calls for the plugin draft

* tiny code revert for unused variable

* add comments and replace some `any`s with `unknown`s for plugin related code

* 5427 contracts unit tests (#5465)

* sample abi of a contract

* unit tests contract

* contract events test

* unit test data

* additional tests for more coverage

* default common check

* test contract source and updated abi

* common type fix

* Merge with 5091

Co-authored-by: Nikos Iliakis <[email protected]>
Co-authored-by: Junaid <[email protected]>
Co-authored-by: Wyatt Barnes <[email protected]>

* Move web3-plugin-example to tools/. Remove @ts-expect-errors. Move Web3 module redeclarations to test files

* Update CHANGELOGs

* Update packages/web3-types/CHANGELOG.md

* Update CHANGELOG.md

Co-authored-by: Muhammad Altabba <[email protected]>

* Revert change to Web3 export

* WIP plugin docs

* Update plugin_authors doc

* Update plugin_users doc

* Rewording

* Update packages/web3/CHANGELOG.md

* Update CHANGELOG.md

* Update docs/docs/guides/web3_plugin_guide/plugin_authors.md

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

* Update docs/docs/guides/web3_plugin_guide/plugin_users.md

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

* Update docs/docs/guides/web3_plugin_guide/plugin_users.md

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

* Apply suggestions from code review

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

* Update tree shaking guide to sidebar position 2

* Update plugin authors sidebar position to 0

* Update docs/docs/guides/web3_plugin_guide/plugin_users.md

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

Co-authored-by: Muhammad Altabba <[email protected]>
Co-authored-by: Nikos Iliakis <[email protected]>
Co-authored-by: Junaid <[email protected]>
Co-authored-by: Alex  <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@spacesailor24 spacesailor24 Awaiting requested review from spacesailor24

3 more reviewers

@avkos avkos avkos approved these changes

@Muhammad-Altabba Muhammad-Altabba Muhammad-Altabba approved these changes

@luu-alex luu-alex luu-alex approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jdevcs jdevcs

Labels

4.x 4.0 related

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jdevcs @avkos @Muhammad-Altabba @luu-alex