Skip to content
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2cd4010
moved proposals tests to constantinople folder
gleb-urvanov Apr 28, 2020
d125bb2
Merge branch 'migration-constantinople' into feature/new-proposals-tests
gleb-urvanov Apr 29, 2020
96bdcec
removed rome tests, introduced assertions for new proposal tests
gleb-urvanov Apr 29, 2020
5527844
validator count proposal test implemented
gleb-urvanov Apr 30, 2020
20ff033
set lead proposal test implemented
gleb-urvanov May 4, 2020
5ac4c2a
build script for network tests added, readme updated
gleb-urvanov May 4, 2020
3c35b7d
Merge branch 'feature/build-script' into feature/proposals-test-coverage
gleb-urvanov May 5, 2020
3e0d381
evict storage provider proposal test implemented
gleb-urvanov May 5, 2020
9328be9
set role parameters test implementation
gleb-urvanov May 6, 2020
8ba80f2
Merge branch 'development' of https://github.com/Joystream/substrate-…
gleb-urvanov May 6, 2020
ad1b0be
Merge branch 'development' into feature/proposals-test-coverage
gleb-urvanov May 6, 2020
52e2dd0
storage provider parameters proposal test
gleb-urvanov May 7, 2020
239c80c
Constantinople proposals covered with network tests
gleb-urvanov May 7, 2020
d2b986d
added periods alteration to the test
gleb-urvanov May 7, 2020
7138f79
composed scenarios as sequence of functions
gleb-urvanov May 18, 2020
387b981
merged development
gleb-urvanov May 19, 2020
644ea80
refactored to use node-tap
gleb-urvanov May 19, 2020
ff42ae6
refactored to use node-tap instead of mocha
gleb-urvanov May 19, 2020
f0795a4
code refactoring, split test code into test and test function impleme…
gleb-urvanov May 20, 2020
2d44c1d
Merge branch 'development' into feature/proposals-test-coverage
gleb-urvanov May 26, 2020
0d95545
testnet script minor fix
gleb-urvanov May 26, 2020
03a16a2
made test implementation less opinionated
gleb-urvanov May 26, 2020
114145f
introduced timeout calculation
gleb-urvanov May 27, 2020
b3f42a6
changed timeout on evict storage provider test, small typo fixed
gleb-urvanov May 28, 2020
28e10a1
assertions refactored
gleb-urvanov May 28, 2020
25b8097
run test chain script fixed to work with mac
gleb-urvanov May 28, 2020
d95bde2
.tmp folder for chainspec
gleb-urvanov May 28, 2020
2bb49e6
Update scripts/run-test-chain.sh
gleb-urvanov May 28, 2020
2f11cb8
reverted accidental chainge
gleb-urvanov May 28, 2020
d4dbc31
code cleaning
gleb-urvanov May 28, 2020
1109153
merged development
gleb-urvanov May 31, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,10 @@ yarn*
.vscode

# Compiled WASM code
*.wasm
*.wasm

# Chain specificaiton
chainspec.json

# Istanbul report output
**.nyc_output/
13 changes: 11 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,23 @@ This will build and run a fresh new local development chain purging existing one
cargo test
```

### API integration tests
### Network tests

```bash
./scripts/run-dev-chain.sh
./scripts/run-test-chain.sh
yarn test
```

To run the integration tests with a different chain, you can omit step running the local development chain and set the node URL using `NODE_URL` environment variable.
Proposal grace periods should be set to 0, otherwise proposal network tests will fail.

### Rome-Constantinople migration network test

Ensure Rome node is up and running, and node URL is set using `NODE_URL` environment variable (default value is `localhost:9944`).

```bash
yarn test-migration
```

## Joystream Runtime

Expand Down
13 changes: 13 additions & 0 deletions scripts/run-test-chain.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/bash
cargo run --release -p joystream-node build-spec --chain dev > chainspec.json
Copy link
Member

Choose a reason for hiding this comment

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

Although this approach is temporary and eventually we will have parameterized build of the runtime, and producing this chainspec file as an artifact of running tests will not be necessary, it should be generated in some temporary location folder, maybe the system temporary path?

This goes for any other temporary artifacts of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, .tmp folder introduced

sed -i 's/"setValidatorCountProposalGracePeriod":.*/"setValidatorCountProposalGracePeriod": 0,/' chainspec.json
sed -i 's/"runtimeUpgradeProposalGracePeriod":.*/"runtimeUpgradeProposalGracePeriod": 0,/' ./chainspec.json
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sed -i 's/"runtimeUpgradeProposalGracePeriod":.*/"runtimeUpgradeProposalGracePeriod": 0,/' ./chainspec.json
sed -i 's/"runtimeUpgradeProposalGracePeriod":.*/"runtimeUpgradeProposalGracePeriod": 0,/' chainspec.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

sed -i 's/"setElectionParametersProposalGracePeriod":.*/"setElectionParametersProposalGracePeriod": 0,/' ./chainspec.json
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sed -i 's/"setElectionParametersProposalGracePeriod":.*/"setElectionParametersProposalGracePeriod": 0,/' ./chainspec.json
sed -i 's/"setElectionParametersProposalGracePeriod":.*/"setElectionParametersProposalGracePeriod": 0,/' chainspec.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

sed -i 's/"textProposalGracePeriod":.*/"textProposalGracePeriod": 0,/' chainspec.json
sed -i 's/"setContentWorkingGroupMintCapacityProposalGracePeriod":.*/"setContentWorkingGroupMintCapacityProposalGracePeriod": 0,/' chainspec.json
sed -i 's/"setLeadProposalGracePeriod":.*/"setLeadProposalGracePeriod": 0,/' chainspec.json
sed -i 's/"spendingProposalGracePeriod":.*/"spendingProposalGracePeriod": 0,/' chainspec.json
sed -i 's/"evictStorageProviderProposalGracePeriod":.*/"evictStorageProviderProposalGracePeriod": 0,/' chainspec.json
sed -i 's/"setStorageRoleParametersProposalGracePeriod":.*/"setStorageRoleParametersProposalGracePeriod": 0/' chainspec.json
yes | cargo run --release -p joystream-node -- purge-chain --dev
cargo run --release -p joystream-node -- --chain=chainspec.json --alice --validator
14 changes: 9 additions & 5 deletions tests/network-tests/.env
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,20 @@ MEMBERSHIP_CREATION_N = 2
# ID of the membership paid terms used in membership creation test.
MEMBERSHIP_PAID_TERMS = 0
# Council stake amount for first K accounts in council election test.
COUNCIL_STAKE_GREATER_AMOUNT = 1500
COUNCIL_STAKE_GREATER_AMOUNT = 3000
# Council stake amount for first the rest participants in council election test.
COUNCIL_STAKE_LESSER_AMOUNT = 1000
COUNCIL_STAKE_LESSER_AMOUNT = 2000
# Number of members with greater stake in council election test.
COUNCIL_ELECTION_K = 2
# Balance to spend using spending proposal
SPENDING_BALANCE = 1000
# Minting capacity for content working group minting capacity test.
MINTING_CAPACITY = 100020
# Minting capacity increment for content working group minting capacity test.
MINTING_CAPACITY_INCREMENT = 20
# Minting capacity for council mint for spending proposal.
COUNCIL_MINTING_CAPACITY = 100000
# Stake amount for Rome runtime upgrade proposal
RUNTIME_UPGRADE_PROPOSAL_STAKE = 100000
# Validator count increment for Validator count test.
VALIDATOR_COUNT_INCREMENT = 2
# Constantinople runtime path
RUNTIME_WASM_PATH = ../../target/release/wbuild/joystream-node-runtime/joystream_node_runtime.compact.wasm
RUNTIME_WASM_PATH = ../../target/release/wbuild/joystream-node-runtime/joystream_node_runtime.compact.wasm
12 changes: 6 additions & 6 deletions tests/network-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
"license": "GPL-3.0-only",
"scripts": {
"build": "tsc --build tsconfig.json",
"test": "mocha -r ts-node/register src/tests/constantinople/*",
"test-migration": "mocha -r ts-node/register src/tests/rome/* && mocha -r ts-node/register src/tests/constantinople/*",
"test": "tap --files ts-node/register src/constantinople/tests/proposals/*Test.ts",
Copy link
Member

Choose a reason for hiding this comment

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

We don't seem to have a way to control the order of tests, as far as I can see they are run in alphabetic order of the file names (not different than mocha).

Given that the test scenarios should be independent, (currently they are all executed one after the other with the same chain so they aren't exactly isolated) the order is not important, but it would still be valuable to control.

The plan of course is to have a better way to setup a network before running the test scenarios, so this is probably the task of the orchestrator/task runner/

"test-migration": "tap --files src/rome/tests/romeRuntimeUpgradeTest.ts --files src/constantinople/tests/electingCouncilTest.ts",
"lint": "tslint --project tsconfig.json",
"prettier": "prettier --write ./src"
},
"dependencies": {
"@joystream/types": "",
"@rome/types@npm:@joystream/types": "^0.7.0",
"@constantinople/types@npm:@joystream/types": "^0.9.1",
"@polkadot/api": "^0.96.1",
"@polkadot/keyring": "^1.7.0-beta.5",
"@rome/types@npm:@joystream/types": "^0.7.0",
"@types/bn.js": "^4.11.5",
"bn.js": "^4.11.8",
"dotenv": "^8.2.0",
Expand All @@ -23,11 +23,11 @@
"devDependencies": {
"@polkadot/ts": "^0.3.14",
"@types/chai": "^4.2.11",
"@types/mocha": "^7.0.2",
"@types/tap": "^14.10.0",
"@types/uuid": "^7.0.2",
"chai": "^4.2.0",
"mocha": "^7.1.1",
"prettier": "2.0.2",
"tap": "^14.10.7",
"ts-node": "^8.8.1",
"tslint": "^6.1.0",
"typescript": "^3.8.3"
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { KeyringPair } from '@polkadot/keyring/types';
import { membershipTest } from './impl/membershipCreation';
import { councilTest } from './impl/electingCouncil';

const m1KeyPairs: KeyringPair[] = new Array();
const m2KeyPairs: KeyringPair[] = new Array();

membershipTest(m1KeyPairs);
membershipTest(m2KeyPairs);
councilTest(m1KeyPairs, m2KeyPairs);
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { membershipTest } from './membershipCreationTest';
import { KeyringPair } from '@polkadot/keyring/types';
import { ApiWrapper } from './utils/apiWrapper';
import { ApiWrapper } from '../../utils/apiWrapper';
import { WsProvider, Keyring } from '@polkadot/api';
import { initConfig } from './utils/config';
import BN = require('bn.js');
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised this syntax ever worked in the first place! :)

import { registerJoystreamTypes, Seat } from '@joystream/types';
import { initConfig } from '../../utils/config';
import BN from 'bn.js';
Copy link
Member

Choose a reason for hiding this comment

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

I've often had typescript linter complain that bn.js doesn't have a default export and so in this import BN would be undefined. Has this been fixed in newer versions of bn.js?

import { registerJoystreamTypes, Seat } from '@constantinople/types';
import { assert } from 'chai';
import { v4 as uuid } from 'uuid';
import { Utils } from './utils/utils';
import { Utils } from '../../utils/utils';
import tap from 'tap';

export function councilTest(m1KeyPairs: KeyringPair[], m2KeyPairs: KeyringPair[]) {
initConfig();
Expand All @@ -17,18 +17,19 @@ export function councilTest(m1KeyPairs: KeyringPair[], m2KeyPairs: KeyringPair[]
const K: number = +process.env.COUNCIL_ELECTION_K!;
const greaterStake: BN = new BN(+process.env.COUNCIL_STAKE_GREATER_AMOUNT!);
const lesserStake: BN = new BN(+process.env.COUNCIL_STAKE_LESSER_AMOUNT!);
const defaultTimeout: number = 120000;
const defaultTimeout: number = 300000;
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why the timeout had to be increased here, and in some other tests also.

Wouldn't it be better if the timeout value for each scenario was derived based on how many finalized blocks the tests should be completed within, and the chain's block interval?

Its also not clear, is this timeout for the entire suite or per .test()? If it is per test then the timeout should be specific to each test, not the max value to accommodate all the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The timeout time now is defined using number of blocks required for the test to pass.

let sudo: KeyringPair;
let apiWrapper: ApiWrapper;

before(async function () {
this.timeout(defaultTimeout);
tap.setTimeout(defaultTimeout);

tap.test('Electing council test setup', async () => {
registerJoystreamTypes();
const provider = new WsProvider(nodeUrl);
apiWrapper = await ApiWrapper.create(provider);
});

it('Electing a council test', async () => {
tap.test('Electing a council test', async () => {
// Setup goes here because M keypairs are generated after before() function
sudo = keyring.addFromUri(sudoUri);
let now = await apiWrapper.getBestBlock();
Expand Down Expand Up @@ -111,17 +112,9 @@ export function councilTest(m1KeyPairs: KeyringPair[], m2KeyPairs: KeyringPair[]
`Member ${seat.member} has unexpected stake ${Utils.getTotalStake(seat)}`
)
);
}).timeout(defaultTimeout);
});

after(() => {
tap.teardown(() => {
apiWrapper.close();
});
}

describe('Council integration tests', () => {
const m1KeyPairs: KeyringPair[] = new Array();
const m2KeyPairs: KeyringPair[] = new Array();
membershipTest(m1KeyPairs);
membershipTest(m2KeyPairs);
councilTest(m1KeyPairs, m2KeyPairs);
});
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { WsProvider } from '@polkadot/api';
import { registerJoystreamTypes } from '@joystream/types';
import { registerJoystreamTypes } from '@constantinople/types';
import { Keyring } from '@polkadot/keyring';
import { assert } from 'chai';
import { KeyringPair } from '@polkadot/keyring/types';
import BN = require('bn.js');
import { ApiWrapper } from './utils/apiWrapper';
import { initConfig } from './utils/config';
import BN from 'bn.js';
import { ApiWrapper } from '../../utils/apiWrapper';
import { initConfig } from '../../utils/config';
import { v4 as uuid } from 'uuid';
import tap from 'tap';

export function membershipTest(nKeyPairs: KeyringPair[]) {
initConfig();
Expand All @@ -15,15 +16,16 @@ export function membershipTest(nKeyPairs: KeyringPair[]) {
const paidTerms: number = +process.env.MEMBERSHIP_PAID_TERMS!;
Copy link
Member

Choose a reason for hiding this comment

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

For all the functions inside the impl folder which are now the reusable unit, it might be better to have these scenario parameters be passed as arguments and the caller would read them from process.env

registerJoystreamTypes() is also called in each test setup, and I think the caller of the test only needs to do that one time, and might as well also create the api instance and pass that as argument as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Scenario parameters now read from environment in scenarios and are passed to the implementation as parameters.

Type registration and api instance are still test-specific. I've run into issues while trying move them up to the scenario level. Let me know if we should discuss it.

const nodeUrl: string = process.env.NODE_URL!;
const sudoUri: string = process.env.SUDO_ACCOUNT_URI!;
const defaultTimeout: number = 30000;
const defaultTimeout: number = 75000;
let apiWrapper: ApiWrapper;
let sudo: KeyringPair;
let aKeyPair: KeyringPair;
let membershipFee: BN;
let membershipTransactionFee: BN;

before(async function () {
this.timeout(defaultTimeout);
tap.setTimeout(defaultTimeout);

tap.test('Membership creation test setup', async () => {
registerJoystreamTypes();
const provider = new WsProvider(nodeUrl);
apiWrapper = await ApiWrapper.create(provider);
Expand All @@ -42,7 +44,7 @@ export function membershipTest(nKeyPairs: KeyringPair[]) {
await apiWrapper.transferBalance(sudo, aKeyPair.address, membershipTransactionFee);
});

it('Buy membeship is accepted with sufficient funds', async () => {
tap.test('Buy membeship is accepted with sufficient funds', async () => {
await Promise.all(
nKeyPairs.map(async (keyPair, index) => {
await apiWrapper.buyMembership(keyPair, paidTerms, `new_member_${index}${keyPair.address.substring(0, 8)}`);
Expand All @@ -53,9 +55,9 @@ export function membershipTest(nKeyPairs: KeyringPair[]) {
.getMemberIds(keyPair.address)
.then(membership => assert(membership.length > 0, `Account ${keyPair.address} is not a member`))
);
}).timeout(defaultTimeout);
});

it('Account A can not buy the membership with insufficient funds', async () => {
tap.test('Account A can not buy the membership with insufficient funds', async () => {
await apiWrapper
.getBalance(aKeyPair.address)
.then(balance =>
Expand All @@ -68,9 +70,9 @@ export function membershipTest(nKeyPairs: KeyringPair[]) {
apiWrapper
.getMemberIds(aKeyPair.address)
.then(membership => assert(membership.length === 0, 'Account A is a member'));
}).timeout(defaultTimeout);
});

it('Account A was able to buy the membership with sufficient funds', async () => {
tap.test('Account A was able to buy the membership with sufficient funds', async () => {
await apiWrapper.transferBalance(sudo, aKeyPair.address, membershipFee.add(membershipTransactionFee));
apiWrapper
.getBalance(aKeyPair.address)
Expand All @@ -81,14 +83,9 @@ export function membershipTest(nKeyPairs: KeyringPair[]) {
apiWrapper
.getMemberIds(aKeyPair.address)
.then(membership => assert(membership.length > 0, 'Account A is a not member'));
}).timeout(defaultTimeout);
});

after(() => {
tap.teardown(() => {
apiWrapper.close();
});
}

describe.skip('Membership integration tests', () => {
const nKeyPairs: KeyringPair[] = new Array();
membershipTest(nKeyPairs);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { KeyringPair } from '@polkadot/keyring/types';
import { membershipTest } from './impl/membershipCreation';

const nKeyPairs: KeyringPair[] = new Array();

membershipTest(nKeyPairs);
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { KeyringPair } from '@polkadot/keyring/types';
import { membershipTest } from '../impl/membershipCreation';
import { councilTest } from '../impl/electingCouncil';
import { electionParametersProposalTest } from './impl/electionParametersProposal';

const m1Keys: KeyringPair[] = new Array();
const m2Keys: KeyringPair[] = new Array();

membershipTest(m1Keys);
membershipTest(m2Keys);
councilTest(m1Keys, m2Keys);
electionParametersProposalTest(m1Keys, m2Keys);
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { KeyringPair } from '@polkadot/keyring/types';
import { membershipTest } from '../impl/membershipCreation';
import { councilTest } from '../impl/electingCouncil';
import { evictStorageProviderTest } from './impl/evictStoraveProvider';

const m1Keys: KeyringPair[] = new Array();
const m2Keys: KeyringPair[] = new Array();

membershipTest(m1Keys);
membershipTest(m2Keys);
councilTest(m1Keys, m2Keys);
evictStorageProviderTest(m1Keys, m2Keys);
Loading