Internal improvement: Move compile-solidity tests to a new separate package #5445

eggplantzzz · 2022-08-16T18:43:01Z

While bootstrapping Truffle, you will notice a warning printed by lerna about a circular dependency. The tests for @truffle/compile-solidity use @truffle/resolver (as it is required in the config for certain flows) and @truffle/resolver has a dependency on @truffle/compile-solidity. To eliminate this circular dependency, these tests have been moved to their own package (and converted to TypeScript for kicks!).

Note that this PR is against the moreTS branch which is the branch where @truffle/compile-solidity is fully converted (almost, not the tests) to TypeScript in this PR.

haltman-at

My main note here is that there are a bunch of places where assert is used in a way that isn't the closest match to the intent, I would suggest changing those. A bunch of those are pre-existing so maybe that's not entirely necessary? But I thought it was worth pointing out at least.

haltman-at · 2022-08-25T04:30:36Z

packages/compile-solidity-tests/test/compileWithPragmaAnalysis.ts

@@ -129,8 +133,11 @@ describe("compileWithPragmaAnalysis", function () {
        options: config,
        paths: [path.join(sourceDirectory, "withImports", "C.sol")]
      });
+      if (!compilations) {


If you're doing if (...) { assert.fail() }, you're not using assertions properly. I think the right way to do this in chai would be assert.isDefined(compilations, "Compilations were not created").

packages/compile-solidity-tests/test/compileWithPragmaAnalysis.ts

packages/compile-solidity-tests/test/compilerSupplier/index.ts

packages/compile-solidity-tests/test/compilerSupplier/loadingStrategies/VersionRange.ts

packages/compile-solidity-tests/test/run.ts

packages/compile-solidity-tests/package.json

eggplantzzz · 2022-08-25T14:19:55Z

Sounds good to me @haltman-at about the massaging of the chai assertions!

eggplantzzz · 2022-08-25T14:34:59Z

@haltman-at it seems like chai has no preference for assert(a === b) or assert.equal(a, b). They have examples of both kinds of usage in the docs https://www.chaijs.com/api/assert/. I changed some of them over. Do you have a preference for assert.equal?

haltman-at · 2022-08-25T21:46:39Z

@haltman-at it seems like chai has no preference for assert(a === b) or assert.equal(a, b). They have examples of both kinds of usage in the docs https://www.chaijs.com/api/assert/. I changed some of them over. Do you have a preference for assert.equal?

Yeah the latter generates much better error messages when it fails.

eggplantzzz · 2022-08-26T08:40:37Z

Oh that's when it gives you the actual values of the things being tested on failure in the test report. That's always super helpful.

packages/compile-solidity-tests/test/compileWithPragmaAnalysis.ts

haltman-at

(Actually, just noticed a few asserts that could use assert.include!)

packages/compile-solidity-tests/test/compilerSupplier/index.ts

packages/compile-solidity-tests/test/supplier.ts

haltman-at

OK, seems good to me, except package versions need to be updated again due to the release last week!

haltman-at · 2022-09-13T20:09:41Z

packages/compile-solidity-tests/package.json

+    "test": "./scripts/test.sh"
+  },
+  "devDependencies": {
+    "@truffle/artifactor": "^4.0.167",


These need to be updated again.

…tions

in build script

eggplantzzz changed the base branch from develop to moreTS August 16, 2022 18:43

eggplantzzz requested review from haltman-at, gnidan, cds-amal, lsqproduction and sukanyaparashar August 16, 2022 18:44

eggplantzzz force-pushed the and-tests branch from 53ef998 to a98d5c1 Compare August 17, 2022 17:17

eggplantzzz force-pushed the moreTS branch from 1c599cf to aabd2d5 Compare August 17, 2022 20:06

eggplantzzz force-pushed the and-tests branch 2 times, most recently from e03d9ac to 81fafbb Compare August 17, 2022 20:14

eggplantzzz mentioned this pull request Aug 17, 2022

Internal improvement: Fully convert compile-solidity to use TypeScript #5432

Merged

eggplantzzz force-pushed the moreTS branch from b42f5ce to 821a878 Compare August 18, 2022 16:08

eggplantzzz force-pushed the and-tests branch from bf3275c to 1adb822 Compare August 18, 2022 16:43

Base automatically changed from moreTS to develop August 23, 2022 17:39

haltman-at reviewed Aug 25, 2022

View reviewed changes

eggplantzzz force-pushed the and-tests branch from 1adb822 to 9ff142f Compare August 25, 2022 14:47

eggplantzzz mentioned this pull request Aug 25, 2022

Fix: official repo to list Solidity's releases #5008

Merged

eggplantzzz force-pushed the and-tests branch from eb4b8b2 to 2a04cd0 Compare August 26, 2022 15:19

haltman-at reviewed Aug 26, 2022

View reviewed changes

packages/compile-solidity-tests/test/compileWithPragmaAnalysis.ts Outdated Show resolved Hide resolved

haltman-at reviewed Aug 26, 2022

View reviewed changes

eggplantzzz force-pushed the and-tests branch from 9348218 to ca728b0 Compare September 13, 2022 18:27

haltman-at reviewed Sep 13, 2022

View reviewed changes

eggplantzzz added 4 commits September 13, 2022 16:27

continue typing

6da9d50

adjust some typings and get rid of config from devDeps

d69e2c9

WIP - convert all tests to TS and start resolving errors

1874dd9

export more stuff from compile-solidity and fix up some of the tests

e208544

eggplantzzz added 15 commits September 13, 2022 16:27

remove plugin that seems unnecessary?

a4e0ab2

add package to ignored list for dep check

e082b66

remove seemingly unneeded package

4f123df

fix an error with prep for a test

d79cce5

add exports for @truffle/compile-solidity

571b170

TS-finesse a test

487f61d

make minor updates to fix a botched rebase

b817799

update some tests

4470313

update versions of truffle packages in compile-solidity-tests

38d1c9a

use more specific assertions in tests

0add028

correct faulty test script search pattern and massage some more asser…

8e0def9

…tions

move ts-ignores in test

0a76815

correct silly eorror

ec24211

in build script

remove unneeded assertion

2898a99

use assert.include in tests

87ee8bc

eggplantzzz force-pushed the and-tests branch from ca728b0 to 87ee8bc Compare September 13, 2022 20:27

bump dev deps to the current package version

57d00cf

haltman-at approved these changes Sep 13, 2022

View reviewed changes

eggplantzzz merged commit 6bc7700 into develop Sep 13, 2022

eggplantzzz deleted the and-tests branch September 13, 2022 21:28

This was referenced Sep 15, 2022

Circular dependency between @truffle/resolver and @truffle/compile-solidity #5244

Closed

Eliminate circular dependency between compile-solidity and resolver #5540

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Internal improvement: Move compile-solidity tests to a new separate package #5445

Internal improvement: Move compile-solidity tests to a new separate package #5445

eggplantzzz commented Aug 16, 2022 •

edited

Loading

haltman-at left a comment

haltman-at Aug 25, 2022 •

edited

Loading

eggplantzzz commented Aug 25, 2022

eggplantzzz commented Aug 25, 2022

haltman-at commented Aug 25, 2022

eggplantzzz commented Aug 26, 2022

haltman-at left a comment

haltman-at left a comment

haltman-at Sep 13, 2022

Internal improvement: Move compile-solidity tests to a new separate package #5445

Internal improvement: Move compile-solidity tests to a new separate package #5445

Conversation

eggplantzzz commented Aug 16, 2022 • edited Loading

haltman-at left a comment

Choose a reason for hiding this comment

haltman-at Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

eggplantzzz commented Aug 25, 2022

eggplantzzz commented Aug 25, 2022

haltman-at commented Aug 25, 2022

eggplantzzz commented Aug 26, 2022

haltman-at left a comment

Choose a reason for hiding this comment

haltman-at left a comment

Choose a reason for hiding this comment

haltman-at Sep 13, 2022

Choose a reason for hiding this comment

eggplantzzz commented Aug 16, 2022 •

edited

Loading

haltman-at Aug 25, 2022 •

edited

Loading