[DO NOT MERGE] swarm-network-rewrite merge to master #171
Conversation
gbalint
force-pushed
the
swarm-network-rewrite
branch
2 times, most recently
from
70d231b
to
c38007a
Compare
janos
force-pushed
the
swarm-network-rewrite
branch
from
c38007a
to
70d231b
Compare
|
Looking at https://travis-ci.org/ethersphere/go-ethereum/jobs/343431371 , I guess we have a deadlock in the |
gbalint
force-pushed
the
swarm-network-rewrite
branch
from
2e6ef1f
to
299d65f
Compare
whisper/whisperv6/peer_test.go
Outdated
| @@ -101,7 +101,7 @@ var debugMode = false | |||
| var prevTime time.Time | |||
| var cntPrev int | |||
|
|
|||
| func TestSimulation(t *testing.T) { | |||
| func XTestSimulation(t *testing.T) { | |||
There was a problem hiding this comment.
@gluk256 the X is disabling tests, similar to t.Skip() - we disabled a few flaky tests on this branch so that we focus on our swarm test failures while we are developing the swarm-network-rewrite project.
Once this gets to merge to ethereum/go-ethereum parent repo, we will revert those, and I believe you probably would have solved any flakes by then.
Add TestNewSwarm and simplify NewSwarm constructor
Set ETag header with correct syntax
swarm/pss: move EnableMetrics away from testutil
master merge to swarm-network-rewrite
swarm/pss/pss_test.go
Outdated
| @@ -951,7 +953,7 @@ func worker(id int, jobs <-chan Job, rpcs map[discover.NodeID]*rpc.Client, pubke | |||
| // nodes/msgs/addrbytes/adaptertype | |||
| // if adaptertype is exec uses execadapter, simadapter otherwise | |||
| func TestNetwork2000(t *testing.T) { | |||
| //testutil.EnableMetrics() | |||
| //enableMetrics() | |||
There was a problem hiding this comment.
Wouldn't that be better with a flag then
There was a problem hiding this comment.
IDEs are hard to configure to use custom flags for every test, I personally find it easier to uncomment the line.
There was a problem hiding this comment.
I am fine with this, but I have had comments on my PRs to have such comments removed. Maybe we should discuss it in a roundtable?
There was a problem hiding this comment.
@holisticode sure, let's discuss this. I agree in general that we should not keep dead code, especially in the production code, however I already removed this once from tests, and then we had to introduce it yet again with @nolash . I also think this specific line will be helpful for other tests in the future.
I have definitely been against dead code in the production code in the past, and if you feel strongly that this is the same, I will be happy to remove it! :)
On a second thought adding it as a flag is also a possibility, I could just have a custom keybinding triggering with the flag :)
There was a problem hiding this comment.
No strong feelings, just aiming at consistent handling.
|
No, we only want metrics when debugging smth, we don't want them on Travis.
…On Thu, Jun 14, 2018, 8:45 PM holisticode ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In swarm/pss/pss_test.go
<#171 (comment)>
:
> @@ -951,7 +953,7 @@ func worker(id int, jobs <-chan Job, rpcs map[discover.NodeID]*rpc.Client, pubke
// nodes/msgs/addrbytes/adaptertype
// if adaptertype is exec uses execadapter, simadapter otherwise
func TestNetwork2000(t *testing.T) {
- //testutil.EnableMetrics()
+ //enableMetrics()
remove comment?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#171 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADFG-9ZU4EY1TKrlRYFw7KPb1Pe4UY-ks5t8qEmgaJpZM4RCleG>
.
|
|
Check: conflicting files it seems |
vendor: remove unused libraries
swarm/api: update log.Output to report meaningful log line
just change the background color as per the suggestion by @homotopyco…
# Conflicts: # bmt/bmt_r.go
Add tests for buffered http get
swarm/pss: Update benchmark tests
swarm-network-rewrite is a long-living development branch which contains the network rewrite project code until it is feature complete.
This PR is just for reference, to see how complex it will be to merge when we are ready.