Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Default Testing network_id to match GanacheUI #1577

Closed
wants to merge 2 commits into from

Conversation

CruzMolina
Copy link
Contributor

@CruzMolina CruzMolina commented Dec 21, 2018

Per DavidBurela's feedback: https://twitter.com/DavidBurela/status/1075961475145949185

Defaults truffle test to use GanacheUI's default network_id when no network configuration is defined. The network will currently be defined as test.

If it's a must for UX/UI reasons to have the network logged as ganache, then probably something along the lines of setting a boolean in options during the workflow and passing that val around will need to be done.

@CruzMolina
Copy link
Contributor Author

Should pass tests and build successfully once the leftover v4 test stuff is updated.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.626% when pulling eb0c00e on fix-networkID into 7ea11ab on develop.

@gnidan
Copy link
Contributor

gnidan commented Jan 2, 2019

(I have a reminder to come up with my thoughts on the requirements for this. Will circle back.)

@CruzMolina
Copy link
Contributor Author

Following up on this, don't recall why it would be an issue to change the network_id to match GanacheUI's, but I'm probably forgetting something.

@eggplantzzz
Copy link
Contributor

@CruzMolina So did we have any followup thoughts on this? It looks like it might be fine?

@CruzMolina
Copy link
Contributor Author

@eggplantzzz This still needs to be smoke-tested. It's been a while, but I believe there are a couple edge cases to consider. Will probably need to document pros/cons of how things work now vs how they work on this branch.

@CruzMolina
Copy link
Contributor Author

So, after QAing this branch a bit, I haven't been able to find any bugs for this.

Current downsides have been well-documented in #1726. i.e. running truffle test out of the box w/ GanacheUI results in:

Error: The network id specified in the truffle config (4447) does not match the one returned by the network (5777).  Ensure that both the network and the provider are properly configured.
    at Object.detectAndSetNetworkId (/Users/cruzmolina/Code/truffle-projects/truffle/packages/truffle-core/lib/environment.js:100:15)
    at process.internalTickCallback (internal/process/next_tick.js:77:7)

Pinging @gnidan before merging since it seems like he had a thought or two about this.

@CruzMolina
Copy link
Contributor Author

Closing this since I believe we don't actually want truffle test to default to GanacheUI. IIRC, truffle test is meant to spin up a private local development testing chain by default. This behavior can be overridden in the config or via truffle test --network someNetwork.

One possible solution is to have truffle test --network ganache automatically connect to GanacheUI's default settings w/o anything needing to be specified in the config. Another is to update init and official truffle boxes to have a ganache network pre-defined w/ GanacheUI's default settings.

@CruzMolina CruzMolina closed this May 20, 2019
@gnidan gnidan deleted the fix-networkID branch June 12, 2019 17:42
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.

4 participants