Skip to content
This repository has been archived by the owner on Mar 3, 2021. It is now read-only.

sequence for tests using CLI #1329

Merged
merged 4 commits into from
Nov 18, 2019
Merged

sequence for tests using CLI #1329

merged 4 commits into from
Nov 18, 2019

Conversation

Aniket-Engg
Copy link
Collaborator

Sequence of tests will be same as they will appear in test contract file. AST is used to ensure this.

Related to #1240

@Aniket-Engg Aniket-Engg changed the title [WIP] sequence for tests using CLI sequence for tests using CLI Nov 15, 2019
export function runTestFiles(filepath: string, isDirectory: boolean, web3: Web3, opts?: object) {
opts = opts || {}
let sourceASTs: any = {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is interesting. Here you create an empty object that you mutate later on. But you do not reassign the value. Which means that you can use const here ;)

@@ -44,7 +53,13 @@ export function runTestFiles(filepath: string, isDirectory: boolean, web3: Web3,
function compile(next: Function) {
compileFileOrFiles(filepath, isDirectory, { accounts }, next)
},
function deployAllContracts (compilationResult, next: Function) {
function deployAllContracts (compilationResult, asts, next: Function) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any tupes for result & AST ? I've some types here for compilation output. I don't know if it can help.

export async function runTestSources(contractSources, versionUrl, usingWorker, testCallback, resultCallback, finalCallback, importFileCb, opts) {
opts = opts || {}
let sourceASTs: any = {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

function deployAllContracts (compilationResult, next) {
function deployAllContracts (compilationResult, asts, next) {
for(const filename in asts)
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

We use the Javascript style for bracket for loops. I'm fine using the C++ version, but if it doesn't make much difference for you I would recommend doing it like that :

for (const filename in asts) {  // Bracket at the end of the line
  if(filename.includes('_test.sol')) sourceASTs[filename] = asts[filename].ast
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

obviously, we should use javascript style

@@ -79,7 +96,7 @@ export async function runTestSources(contractSources, versionUrl, usingWorker, t
}

async.eachOfLimit(contractsToTest, 1, (contractName: string, index: string | number, cb: ErrorCallback) => {
runTest(contractName, contracts[contractName], contractsToTestDetails[index], { accounts }, _testCallback, (err, result) => {
runTest(contractName, contracts[contractName], contractsToTestDetails[index], sourceASTs[contracts[contractName]['filename']], { accounts }, _testCallback, (err, result) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

To make it more readable you can create variables :

const contract = contracts[contractName];
const details = contractsToTestDetails[index];
const iAmNotSure = sourceASTs[contracts[contractName]['filename']]
runTest(contractName, contract, details, iAmNotSure, ...)

*/

function getTestFunctionsInterface (jsonInterface: any, funcList: string[]) {
let functionsInterface: any[] = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above about const

const specialFunctions = ['beforeAll', 'beforeEach', 'afterAll', 'afterEach']
for(const func of funcList){
if(!specialFunctions.includes(func))
functionsInterface.push(jsonInterface.find(node => node.type === 'function' && node.name === func))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Create a variable for jsonInterface.find(node => node.type === 'function' && node.name === func) with a name that describe what it is.

@Aniket-Engg Aniket-Engg merged commit ddc8303 into master Nov 18, 2019
@Aniket-Engg Aniket-Engg deleted the tests_sequence branch November 18, 2019 05:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants