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 build command #4848

Closed
wants to merge 9 commits into from
Closed

Conversation

wbt
Copy link
Contributor

@wbt wbt commented Mar 14, 2022

Description

This partially addresses long-broken parts of the npm testing scripts this PR template requires that contributors complete.
It doesn't fully address all the issues (e.g. that docker is an undocumented dependency required to run the geth instamine script which is triggered by one of the commands below, but things get much farther along than they used to.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules. [There are none]
  • I ran npm run dtslint with success and extended the tests and types if necessary.
  • I ran npm run test:unit with success.
  • I ran npm run test:cov and my test cases cover all the lines and branches of the added code. [This works better now, but still not 100%]
  • I ran npm run build and tested dist/web3.min.js in a browser.
  • I have tested my code on the live network.
  • I have checked the Deploy Preview and it looks correct. [I'm not sure what this refers to or how to do this; further instructions could be helpful.]
  • I have updated the CHANGELOG.md file in the root folder.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1983218666

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 429 unchanged lines in 7 files lost coverage.
  • Overall coverage decreased (-2.3%) to 72.688%

Files with Coverage Reduction New Missed Lines %
packages/web3-core-requestmanager/src/jsonrpc.js 1 88.0%
packages/web3-core-helpers/src/formatters.js 25 81.07%
packages/web3-core-helpers/src/errors.js 31 4.41%
packages/web3-utils/src/soliditySha3.js 55 5.13%
packages/web3-utils/src/index.js 62 29.31%
packages/web3-utils/src/utils.js 92 12.86%
packages/web3-eth-accounts/src/index.js 163 23.77%
Totals Coverage Status
Change from base Build 1975083195: -2.3%
Covered Lines: 3383
Relevant Lines: 4397

💛 - Coveralls

@wbt
Copy link
Contributor Author

wbt commented Mar 15, 2022

The e2e_windows CI step is broken due to the same inability to connect to Infura that holds up other PRs.
The unit_and_e2e_clients CI step failed due to a short timeout that appears to be just unreliability in tests, where random failures of that type are observed locally and usually fixed by re-running the test, not something I have permissions to do in this repository. The coveralls report seems broken, falsely reporting lines as losing coverage when this PR shouldn't affect that metric.

@jdevcs jdevcs added the 1.x 1.0 related issues label Mar 16, 2022
@jdevcs jdevcs added the Review Needed Maintainer(s) need to review label Mar 16, 2022
@spacesailor24 spacesailor24 changed the base branch from 1.x to wyatt/1.x/fix-build-command March 17, 2022 07:17
@spacesailor24 spacesailor24 deleted the branch web3:wyatt/1.x/fix-build-command March 17, 2022 07:27
@spacesailor24
Copy link
Contributor

This somehow was automatically closed, but #4861 supersedes this anyways

@wbt wbt mentioned this pull request Apr 12, 2022
@jdevcs jdevcs mentioned this pull request May 17, 2022
@jdevcs jdevcs mentioned this pull request Jul 15, 2022
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 Review Needed Maintainer(s) need to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants