Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix e2e lerna publish (#3) #3170

Merged
merged 1 commit into from
Oct 31, 2019
Merged

Fix e2e lerna publish (#3) #3170

merged 1 commit into from
Oct 31, 2019

Conversation

cgewecke
Copy link
Collaborator

(DO NOT MERGE until making suggested change in the comment below)

Unfortunately #3157 was only ~%25 correct :/

PR fixes several problems:

  • The packages were not being published by the lerna command correctly - they used exec (obviously wrong). I've had to upgrade Lerna to 3.x to get this to work.
  • Unexpectedly, truffle/contract mostly uses a Web3 from a different truffle module than itself, even though Web3 is a direct dependency there. Both modules need to be modified to get the tests to consume the virtual publication correctly

I've added a 'proof' that this works - there's an incorrect console.log line in web3-core-helpers which is printed out while truffle's tests run. So we know their code is exercising the published copy.
It needs to be removed before merging..

Lastly, some of Truffle's tests are failing with an error that's already been reported at truffle 2512. I don't think any recent changes here are the cause because they have not updated to 1.2.2 yet.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 84.011% when pulling f46237f on cgewecke:tests/fix-npm-publish into 266741c on ethereum:1.x.

@@ -219,6 +219,7 @@ var outputTransactionReceiptFormatter = function (receipt){
if(typeof receipt.status !== 'undefined' && receipt.status !== null) {
receipt.status = Boolean(parseInt(receipt.status));
}
console.log('using new web3 at to format transaction receipt')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Edit this file to remove this line. It proves that the bundle is being published/used correctly in travis here.

@cgewecke cgewecke requested a review from nivida October 31, 2019 03:19
@nivida nivida added 1.x 1.0 related issues CI labels Oct 31, 2019
Copy link
Contributor

@nivida nivida left a comment

Choose a reason for hiding this comment

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

The truffle e2e container is failing during the bootstrapping process:

Bildschirmfoto 2019-10-31 um 12 31 41

@nivida
Copy link
Contributor

nivida commented Oct 31, 2019

@cgewecke The whole e2e tests concept should probably be documented somewhere here in the repository otherwise is it hard to follow up our current thoughts in some months (I will add a build pipeline section to the contributor's section of the readme in #3160 as well).

@cgewecke
Copy link
Collaborator Author

The whole e2e tests concept should probably be documented somewhere here in the repository otherwise is it hard to follow up our current thoughts in some months (I will add a build pipeline section to the contributor's section of the readme in #3160 as well

I agree, this is a good idea.


fi

git checkout $BRANCH --
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The build is failing here because

  • Lerna [won't publish to the virtual registry][1] when the git head is in a detached state.
  • It's necessary to explicitly checkout a branch to trick it
  • The branch isn't present when a fork's changes are proposed for the upstream

I'm going to resolve this temporarily by merging into a staging branch at web3 to make sure the CI runs as expected here. It would be nice if this ran for outside contributors, but perhaps is not critical that it does either because this kind of test is more of a pre-publication tests that's use internally anyway.

lerna/lerna#1595 (comment)

@cgewecke cgewecke changed the base branch from 1.x to tests/e2e-publish-fix-web3 October 31, 2019 17:00
@cgewecke cgewecke merged commit 2c24d35 into web3:tests/e2e-publish-fix-web3 Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants