Typescript test refactoring by crystalin · Pull Request #364 · moonbeam-foundation/moonbeam

crystalin · 2021-04-19T05:01:22Z

Refactoring of the typescript sections:

Moved contracts and utility files in their own folder
Removed "spec" requirement for running the dev node (was not used)
Added support for multiple describeDevMoonbeam (running sequentially in same file)
Moved common function into util folder
Renamed files to be more consistent
Made all tests independent (except those that are not mutating the chain)
Rewrote all contracts into their solidity equivalent (Missing trace tests ones: callee, caller,...)
Rewrote test names to be consistent with ("should perform an action" type)

…orts

…inalizeBlock

…nd nonces in test-txpool

…nonce

…ilters from test to test

…o avoid non-determinism of the nonce

…solc

joelamouche · 2021-04-19T11:58:54Z

tests/tests/test-nonce.ts

-    // For some reason fees are not well estimated otherwise
-    await createAndFinalizeBlock(context.polkadotApi);
+describeDevMoonbeam("Nonce - Initial", (context) => {
+  // before("Setup: Create block with transfer", async () => {


leftover comment!

joelamouche · 2021-04-19T12:01:16Z

tests/util/dev-node.ts

+  nodeStarted = true;
+  const { p2pPort, rpcPort, wsPort } = await findAvailablePorts();
+
+  // console.log("Using ports", { p2pPort, rpcPort, wsPort });


JoshOrndorff

✔️ Awesome!

Our tests are so nice and clean now!

I was a little late on the review, but left some comments for possible minor improvements.

JoshOrndorff · 2021-04-19T13:16:18Z

.github/workflows/release.yml

          cd ../tests
          npm install
-          npm run test;
+          npm run test -- -j $(($(nproc) / 2));


A comment would be nice here. Is this detecting the number of cpus available and allocating half of them to the tests?

JoshOrndorff · 2021-04-19T13:18:46Z

tests/pre-build-contracts.ts

-  await Promise.all(Object.keys(contractSources).map(compile));
+  for (let name of Object.keys(contractSources)) {
+    console.log(`Compiling ${name}`);
+    await compile(name);
+  }


Doesn't this switch from parallel compilation to serial compilation? Is that intentional?

JoshOrndorff · 2021-04-19T13:24:29Z

tests/tests/test-block-1.ts

+    // Originally ,this test required the timestamp be in the last finve minutes.
+    // This requirement doesn't make sense when we forge timestamps in manual seal.


We no longer forge timestamps so we could bring back that lower bound that was previously in Frontier if we want to.

https://github.com/paritytech/frontier/pull/170/files#diff-b097ce1343f1eba88e98eb8392348a53c4f2824ae98a75adbd02120570f5768aL63-L64

Also, there is a typo in that comment. finve -> five

JoshOrndorff · 2021-04-19T13:26:10Z

tests/tests/test-contract-creation.ts

+  it("should store the code on chain", async function () {
+    const { contract, rawTx } = await createContract(context.web3, "TestContract");
+    const { txResults } = await context.createBlock({ transactions: [rawTx] });
+
+    expect(await context.web3.eth.getCode(contract.options.address)).to.deep.equal(
+      "0x608060405234801561001057600080fd5b506004361061002b5760003560e01c8063c6888fa114610030575b" +
+        "600080fd5b61004a6004803603810190610045919061008b565b610060565b60405161005791906100c3565b" +
+        "60405180910390f35b600060078261006f91906100de565b9050919050565b60008135905061008581610171" +
+        "565b92915050565b60006020828403121561009d57600080fd5b60006100ab84828501610076565b91505092" +
+        "915050565b6100bd81610138565b82525050565b60006020820190506100d860008301846100b4565b929150" +
+        "50565b60006100e982610138565b91506100f483610138565b9250817fffffffffffffffffffffffffffffff" +
+        "ffffffffffffffffffffffffffffffffff048311821515161561012d5761012c610142565b5b828202905092" +
+        "915050565b6000819050919050565b7f4e487b71000000000000000000000000000000000000000000000000" +
+        "00000000600052601160045260246000fd5b61017a81610138565b811461018557600080fd5b5056fea26469" +
+        "70667358221220a82dff050f5e40b874671c1f40e579b5a8c361f5313d1a9d32437222ab6a384c64736f6c63" +
+        "430008030033"
+    );
+  });


Should this code literal be imported from somewhere?

JoshOrndorff · 2021-04-19T13:27:17Z

tests/tests/test-contract-loop-cost.ts

+[
+  {
+    loop: 1,
+    gas: 42889,
+  },
+  {
+    loop: 500,
+    gas: 1064354,
+  },
+  {
+    loop: 600,
+    gas: 1269054,
+  },


Maybe comment how we calculated these test vectors?

JoshOrndorff · 2021-04-19T13:34:40Z

tests/util/dev-node.ts

+  // console.log("Using ports", { p2pPort, rpcPort, wsPort });
+  const cmd = BINARY_PATH;
+  const args = [
+    `--execution=Native`, // Faster execution using native


Faster is great. But our docs tell our node operators to run --execution=wasm so in that sense we're testing a different thing that we expect the users to use.

JoshOrndorff · 2021-04-19T13:35:05Z

tests/util/providers.ts

+import { BlockHeader } from "web3-eth";
+import { Log } from "web3-core";
+
+export async function customWeb3Request(web3: Web3, method: string, params: any[]) {


description in docs

JoshOrndorff · 2021-04-19T13:36:01Z

tests/util/setup-dev-tests.ts

+      this.timeout(SPAWNING_TIME);
+      const init = await startMoonbeamDevNode();
+      // Context is given prior to this assignement, so doing
+      // context = init.context will fail because it replace the variable;


s/replace/shadows

JoshOrndorff · 2021-04-19T13:36:18Z

tests/util/setup-dev-tests.ts

+    });
+
+    after(async function () {
+      // console.log(`\x1b[31m Killing RPC\x1b[0m`);


JoshOrndorff · 2021-04-19T13:37:26Z

tests/util/transactions.ts

+  debug("Sending transaction", {
+    ...data,
+    data: !data.data
+      ? ""
+      : data.data.length < 80
+      ? data.data
+      : data.data.substr(0, 7) + "..." + data.data.substr(data.data.length - 5),
+  });


This is a string shortener, right? I wonder if it should be a helper function or have a comment.

* Crude attempt at finding open ports (#346) * added example test for frontier (#343) * added example test for frontier * added npm run non-ci-test * move staking test spec to node/chainspec (#335) * init * hide staking test spec behind test-staking feature flag * clean * comment staking test genesis config for readability * move test spec to separate file * Crude attempt at finding open ports * npx -w * Add missing file * Adds random port and parallel test execution * Fixed ethers test * Adds delay to node start in test Co-authored-by: Antoine Estienne <estienne.antoine@gmail.com> Co-authored-by: Amar Singh <asinghchrony@protonmail.com> Co-authored-by: Crystalin <alan@purestake.com> * Crystalin testweek watch (#347) * Crude attempt at finding open ports * npx -w * Add missing file * Adds random port and parallel test execution * Fixed ethers test * Adds delay to node start in test * Adds support for test watch * Cleaned package.json * Prevent test to run at start of watch * Restore optimization for cargo build Co-authored-by: Stephen Shelton <steve@brewcraft.org> * Unskip tests (#357) * Unskip `fetch genesis block by hash` * Remove timeouts * Unskip block gas limit tests * Remove `opt-level = 0` from Cargo.toml * Unskip * Test contract factory (#351) * added deployContractByName and contractSources * added getcompiled function * increase timeout for testfilterapi * change contract name and add sourcecode * Fix fixture requirement * update package and json Co-authored-by: Crystalin <alan@purestake.com> * Gorka remove intra test dependencies (#355) * Crude attempt at finding open ports * npx -w * Add missing file * Adds random port and parallel test execution * Fixed ethers test * Adds delay to node start in test * Adds support for test watch * Cleaned package.json * Prevent test to run at start of watch * Restore optimization for cargo build * Allow for non-finalization and ParentHash specification in createAndFinalizeBlock * Make test-block tests non-dependent by using non-finalized chains * Separate tests to make them non-dependent and parametrize tx hashes and nonces in test-txpool * it instead of step in test-version * setup providers before tests in test-ethers.ts * Create a block before tests to avoid one depend on the other in test-nonce * it for step in test-receipt * import constants for test-staking * Attemp at parametrizing test-filter-api tests * attemp at parametrizing test-revert-receipt * remove mocha-steps from tests * make test-subscription tests non-dependent * comply with prettier * Add support for retrieving the hash of the generated block * Make test-trace-filter tests non-dependent * balance-tests non-dependent * Non-dependent tests for test-polkadot-api * Add test-fork-chain tests * remove unnecesary timeouts * Use context provider for test-ether.ts * Remove currentId from test-filter-api since we are uninstalling all filters from test to test * Join balance test setups and check balance at each block * Rename variables to camelCase in tests * small introduced bug in test-pool-pending when camelCasing variables * Use a different account for test-revert-receipt deployment contract to avoid non-determinism of the nonce * shorten line because of editconfig * Separate test-subscription and test-subscription-past-events.ts * remove single letter variable in test-filter-api * Small cosmetic changes * revert changes in package.json * Remove beforeEach and substitute it for common function * change var for let or const Co-authored-by: Stephen Shelton <steve@brewcraft.org> Co-authored-by: Crystalin <alan@purestake.com> * Crystalin remove solc (#362) * Crude attempt at finding open ports * npx -w * Add missing file * Adds random port and parallel test execution * Fixed ethers test * Adds delay to node start in test * Adds support for test watch * Cleaned package.json * Prevent test to run at start of watch * Restore optimization for cargo build * Allow for non-finalization and ParentHash specification in createAndFinalizeBlock * Make test-block tests non-dependent by using non-finalized chains * Separate tests to make them non-dependent and parametrize tx hashes and nonces in test-txpool * it instead of step in test-version * setup providers before tests in test-ethers.ts * Create a block before tests to avoid one depend on the other in test-nonce * it for step in test-receipt * import constants for test-staking * Attemp at parametrizing test-filter-api tests * attemp at parametrizing test-revert-receipt * remove mocha-steps from tests * make test-subscription tests non-dependent * comply with prettier * Add support for retrieving the hash of the generated block * Make test-trace-filter tests non-dependent * balance-tests non-dependent * Non-dependent tests for test-polkadot-api * Add test-fork-chain tests * remove unnecesary timeouts * Use context provider for test-ether.ts * Remove currentId from test-filter-api since we are uninstalling all filters from test to test * Join balance test setups and check balance at each block * Rename variables to camelCase in tests * small introduced bug in test-pool-pending when camelCasing variables * Use a different account for test-revert-receipt deployment contract to avoid non-determinism of the nonce * shorten line because of editconfig * Separate test-subscription and test-subscription-past-events.ts * remove single letter variable in test-filter-api * Small cosmetic changes * revert changes in package.json * Remove beforeEach and substitute it for common function * change var for let or const * Removes solc from tests * Fixes path for contract tests * Adds formatting for compiled contracts Co-authored-by: Stephen Shelton <steve@brewcraft.org> Co-authored-by: Gorka Irazoqui <gorka.irazoki@gmail.com> * Typescript test refactoring (#364) * Crude attempt at finding open ports * npx -w * Add missing file * Adds random port and parallel test execution * Fixed ethers test * Adds delay to node start in test * Adds support for test watch * Cleaned package.json * Prevent test to run at start of watch * Restore optimization for cargo build * Allow for non-finalization and ParentHash specification in createAndFinalizeBlock * Make test-block tests non-dependent by using non-finalized chains * Separate tests to make them non-dependent and parametrize tx hashes and nonces in test-txpool * it instead of step in test-version * setup providers before tests in test-ethers.ts * Create a block before tests to avoid one depend on the other in test-nonce * it for step in test-receipt * import constants for test-staking * Attemp at parametrizing test-filter-api tests * attemp at parametrizing test-revert-receipt * remove mocha-steps from tests * make test-subscription tests non-dependent * comply with prettier * Add support for retrieving the hash of the generated block * Make test-trace-filter tests non-dependent * balance-tests non-dependent * Non-dependent tests for test-polkadot-api * Add test-fork-chain tests * remove unnecesary timeouts * Use context provider for test-ether.ts * Remove currentId from test-filter-api since we are uninstalling all filters from test to test * Join balance test setups and check balance at each block * Rename variables to camelCase in tests * small introduced bug in test-pool-pending when camelCasing variables * Use a different account for test-revert-receipt deployment contract to avoid non-determinism of the nonce * shorten line because of editconfig * Separate test-subscription and test-subscription-past-events.ts * remove single letter variable in test-filter-api * Small cosmetic changes * revert changes in package.json * Remove beforeEach and substitute it for common function * change var for let or const * Removes solc from tests * Fixes path for contract tests * Adds formatting for compiled contracts * Refactor tests * Fix formatting * Limit concurrency to half cpus for ts tests * Fixes github action typo Co-authored-by: Stephen Shelton <steve@brewcraft.org> Co-authored-by: Gorka Irazoqui <gorka.irazoki@gmail.com> * Antoine update testweek readme typos (#369) * added instructions to readme * update package lock * Add missing contracts to definition (#368) * Add Callee, Caller, Incrementer * Update test-trace * Remove unused * Move blockscout tracer to util/tracer * Test for txpool multiple transactions (#367) * Improve pending pool test * Rephrase comment * Better naming for txpool tests * Split txpool multiple test for more independance * Fixes few test expectations * Adds test for future transaction * Rename eth pool correctly * Fixes node listeners Co-authored-by: tgmichel <telmo@purestake.com> * Fixes typo in variable name * Added test for block gas in smart contract (#370) * VSCode debugger (support for TS tests) (#348) Add a VSCode debugger config with CodeLLDB. TS tests can be debugged by first starting the node in the debugger, then launching DEBUG_MODE=true npm run test-single in the test folder (modify package.json to change the test file). * Adds test for filter trace pagination (#373) * Fixes trace filter test Co-authored-by: Stephen Shelton <steve@brewcraft.org> Co-authored-by: Antoine Estienne <estienne.antoine@gmail.com> Co-authored-by: Amar Singh <asinghchrony@protonmail.com> Co-authored-by: tgmichel <telmo@purestake.com> Co-authored-by: girazoki <gorka.irazoki@gmail.com> Co-authored-by: nanocryk <6422796+nanocryk@users.noreply.github.com>

notlesh and others added 30 commits April 12, 2021 12:58

Crude attempt at finding open ports

d306e78

npx -w

9af4a89

Add missing file

5cf37a0

Adds random port and parallel test execution

1f80688

Fixed ethers test

b193e1f

Adds delay to node start in test

1614a49

Merge remote-tracking branch 'origin/master' into notlesh-find-open-p…

2e852ea

…orts

Adds support for test watch

1384de8

Merge branch 'testweek' into crystalin-testweek-watch

4b3a85d

Cleaned package.json

1c8e09f

Prevent test to run at start of watch

ffc5135

Restore optimization for cargo build

78552d8

Allow for non-finalization and ParentHash specification in createAndF…

67c971b

…inalizeBlock

Make test-block tests non-dependent by using non-finalized chains

9bf2d45

Separate tests to make them non-dependent and parametrize tx hashes a…

165c1c1

…nd nonces in test-txpool

it instead of step in test-version

5a1492e

setup providers before tests in test-ethers.ts

7836dc6

Create a block before tests to avoid one depend on the other in test-…

51ee9c4

…nonce

it for step in test-receipt

b1d8f02

import constants for test-staking

22b2077

Attemp at parametrizing test-filter-api tests

2c9e87e

attemp at parametrizing test-revert-receipt

b2121eb

remove mocha-steps from tests

adaa05e

make test-subscription tests non-dependent

f083dd6

comply with prettier

aa65135

Add support for retrieving the hash of the generated block

0752113

Make test-trace-filter tests non-dependent

a3bae77

balance-tests non-dependent

9b5d3b6

Non-dependent tests for test-polkadot-api

61e78aa

Add test-fork-chain tests

42e79b9

girazoki and others added 23 commits April 15, 2021 16:55

remove unnecesary timeouts

0a046f4

Use context provider for test-ether.ts

34607b5

Remove currentId from test-filter-api since we are uninstalling all f…

8db05cf

…ilters from test to test

Join balance test setups and check balance at each block

9cb5a43

Rename variables to camelCase in tests

1a54e7d

small introduced bug in test-pool-pending when camelCasing variables

5786d28

Use a different account for test-revert-receipt deployment contract t…

b65f6d5

…o avoid non-determinism of the nonce

shorten line because of editconfig

0876554

Separate test-subscription and test-subscription-past-events.ts

66f3f6c

remove single letter variable in test-filter-api

0e923fe

Small cosmetic changes

611df42

revert changes in package.json

6ee7e5a

Remove beforeEach and substitute it for common function

4305984

change var for let or const

5870a7b

Merge branch 'testweek' into gorka_remove_intra_test_dependencies

6ecef94

Removes solc from tests

8ecddf9

Fixes path for contract tests

1525e88

Adds formatting for compiled contracts

5e8ec5b

Merge remote-tracking branch 'origin/testweek' into crystalin-remove-…

c047861

…solc

Refactor tests

fc2a7af

Fix formatting

339bbf6

Limit concurrency to half cpus for ts tests

51badda

Fixes github action typo

0019667

crystalin merged commit f875f6b into testweek Apr 19, 2021

crystalin deleted the crystalin-clean-tests branch April 19, 2021 11:58

joelamouche reviewed Apr 19, 2021

View reviewed changes

JoshOrndorff reviewed Apr 19, 2021

View reviewed changes

JoshOrndorff mentioned this pull request Apr 19, 2021

Add missing contracts to definition #368

Merged

		// Originally ,this test required the timestamp be in the last finve minutes.
		// This requirement doesn't make sense when we forge timestamps in manual seal.

Conversation

crystalin commented Apr 19, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoshOrndorff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants