Skip to content

Commit

Permalink
Don't use mycelo for e2e tests by default. (#8291)
Browse files Browse the repository at this point in the history
### Description

Following up on #8086, we realized that using mycelo for the tests by default introduces friction for developers making changes to the core contracts, since it requires them to also update the migrations which mycelo uses, which are in the celo-blockchain repo and in Go rather than javascript.  So as a compromise, this PR modifies the tests back to not use mycelo by default.  Instead, they can be switched by devs to use mycelo temporarily (for local runs) as desired, or forced to use mycelo by setting an env variable:

```
export E2E_TESTS_FORCE_USE_MYCELO=true
```

We expect that for the celo-blockchain CI we will set this environment variable, but we won't for the monorepo CI.

### Tested

Verified that the tests now don't use mycelo, but do if you specify the env variable to be true.
  • Loading branch information
Or Neeman authored and yorhodes committed Aug 11, 2021
1 parent 44ff94a commit 8b0a2f7
Show file tree
Hide file tree
Showing 10 changed files with 17 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('Blockchain parameters tests', function (this: any) {
let parameters: BlockchainParametersWrapper

const gethConfig: GethRunConfig = {
useMycelo: true,
migrate: true,
runPath: TMP_PATH,
keepData: false,
networkId: 1101,
Expand Down
2 changes: 1 addition & 1 deletion packages/celotool/src/e2e-tests/cip35_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ function generateTestCases(cipIsActivated: boolean) {
function getGethRunConfig(withDonut: boolean): GethRunConfig {
console.log('getGethRunConfig', withDonut)
return {
useMycelo: true,
migrate: true,
runPath: TMP_PATH,
keepData: false,
networkId: 1101,
Expand Down
2 changes: 1 addition & 1 deletion packages/celotool/src/e2e-tests/governance_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ describe('governance tests', () => {
const gethConfig: GethRunConfig = {
runPath: TMP_PATH,
verbosity: 3,
useMycelo: true,
migrate: true,
networkId: 1101,
network: 'local',
genesisConfig: {
Expand Down
2 changes: 1 addition & 1 deletion packages/celotool/src/e2e-tests/replica_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const verbose = false

describe('replica swap tests', () => {
const gethConfig: GethRunConfig = {
useMycelo: true,
migrate: true,
runPath: TMP_PATH,
verbosity: 4,
networkId: 1101,
Expand Down
2 changes: 1 addition & 1 deletion packages/celotool/src/e2e-tests/slashing_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('slashing tests', function (this: any) {
network: 'local',
networkId: 1101,
runPath: TMP_PATH,
useMycelo: true,
migrate: true,
genesisConfig: {
churritoBlock: 0,
donutBlock: 0,
Expand Down
1 change: 0 additions & 1 deletion packages/celotool/src/e2e-tests/sync_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ describe('sync tests', function (this: any) {
churritoBlock: 0,
donutBlock: 0,
},
useMycelo: true,
instances: [
{
name: 'validator0',
Expand Down
2 changes: 1 addition & 1 deletion packages/celotool/src/e2e-tests/transfer_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ describe('Transfer tests', function (this: any) {

const syncModes = ['full', 'fast', 'light', 'lightest']
const gethConfig: GethRunConfig = {
useMycelo: true,
migrate: true,
networkId: 1101,
network: 'local',
runPath: TMP_PATH,
Expand Down
12 changes: 9 additions & 3 deletions packages/celotool/src/e2e-tests/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { join as joinPath, resolve as resolvePath } from 'path'
import readLastLines from 'read-last-lines'
import Web3 from 'web3'
import { spawnCmd, spawnCmdWithExitOnFailure } from '../lib/cmd-utils'
import { envVar, fetchEnvOrFallback } from '../lib/env-utils'
import {
AccountType,
getPrivateKeysFor,
Expand Down Expand Up @@ -34,6 +35,7 @@ import {
import { GethInstanceConfig } from '../lib/interfaces/geth-instance-config'
import { GethRepository } from '../lib/interfaces/geth-repository'
import { GethRunConfig } from '../lib/interfaces/geth-run-config'
import { stringToBoolean } from '../lib/utils'

const MonorepoRoot = resolvePath(joinPath(__dirname, '../..', '../..'))
const verboseOutput = false
Expand Down Expand Up @@ -215,6 +217,10 @@ export function getHooks(gethConfig: GethRunConfig) {
}

export function getContext(gethConfig: GethRunConfig, verbose: boolean = verboseOutput) {
// Use of mycelo can be enabled through gethConfig or through an env variable
const useMycelo =
!!gethConfig.useMycelo ||
stringToBoolean(fetchEnvOrFallback(envVar.E2E_TESTS_FORCE_USE_MYCELO, 'false'))
const validatorInstances = gethConfig.instances.filter((x: any) => x.validating)

const numValidators = validatorInstances.length
Expand All @@ -240,7 +246,7 @@ export function getContext(gethConfig: GethRunConfig, verbose: boolean = verbose
await checkoutGethRepo(repo.branch || 'master', repo.path)
}

if (gethConfig.useMycelo) {
if (useMycelo) {
await buildGethAll(repo.path)
} else {
await buildGeth(repo.path)
Expand All @@ -255,7 +261,7 @@ export function getContext(gethConfig: GethRunConfig, verbose: boolean = verbose
fs.mkdirSync(gethConfig.runPath, { recursive: true })
}

if (gethConfig.useMycelo) {
if (useMycelo) {
// Compile the contracts first because mycelo assumes they are compiled already, unless told not to
if (!gethConfig.myceloSkipCompilingContracts) {
await spawnCmdWithExitOnFailure('yarn', ['truffle', 'compile'], {
Expand Down Expand Up @@ -312,7 +318,7 @@ export function getContext(gethConfig: GethRunConfig, verbose: boolean = verbose
}
}

if (gethConfig.useMycelo || !(gethConfig.migrate || gethConfig.migrateTo)) {
if (useMycelo || !(gethConfig.migrate || gethConfig.migrateTo)) {
// Just need to initialize the nodes in this case. No need to actually start the network
// since we don't need to run the migrations against it.
for (const instance of gethConfig.instances) {
Expand Down
2 changes: 1 addition & 1 deletion packages/celotool/src/e2e-tests/validator_order_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('governance tests', () => {
networkId: 1101,
network: 'local',
runPath: TMP_PATH,
useMycelo: true,
migrate: true,
instances: _.range(VALIDATORS).map((i) => ({
name: `validator${i}`,
validating: true,
Expand Down
1 change: 1 addition & 0 deletions packages/celotool/src/lib/env-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export enum envVar {
CONSENSUS_TYPE = 'CONSENSUS_TYPE',
CONTEXTS = 'CONTEXTS',
DONUT_BLOCK = 'DONUT_BLOCK',
E2E_TESTS_FORCE_USE_MYCELO = 'E2E_TESTS_FORCE_USE_MYCELO',
EKSPORTISTO_DOCKER_IMAGE_REPOSITORY = 'EKSPORTISTO_DOCKER_IMAGE_REPOSITORY',
EKSPORTISTO_DOCKER_IMAGE_TAG = 'EKSPORTISTO_DOCKER_IMAGE_TAG',
EKSPORTISTO_SUFFIX = 'EKSPORTISTO_SUFFIX',
Expand Down

0 comments on commit 8b0a2f7

Please sign in to comment.