-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
test.replication: remove "down-server" #8715
base: master
Are you sure you want to change the base?
Conversation
496efb4
to
c938fc1
Compare
The naming of the replication test which accessed the "down server" implies it's attempting to connect to a server which is down. The test actually passed for any non-pouchdb server, including: * a hostname which doesn't resolve * a hostname which resolves but server does not respond * a server which responds, but not as a couch server
7edb5d7
to
eaffa24
Compare
@@ -4248,6 +4248,8 @@ adapters.forEach(function (adapters) { | |||
// This test only needs to run for one configuration, and it slows stuff | |||
// down | |||
describe('suite2 test.replication.js-down-test', function () { | |||
this.timeout(360000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weirdly this is necessary, despite the 10ms timeouts set below. Maybe a bug?
@@ -4248,6 +4248,8 @@ adapters.forEach(function (adapters) { | |||
// This test only needs to run for one configuration, and it slows stuff | |||
// down |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But... it's currently run for quite a few configurations. And behaves very differently with different configurations. Perhaps this comment should be updated.
if you look at the down server script, this is especially a server that returns a 500 error. I’m not sure removing this tests helps, but I recall we got here because this used to rely on an external public server |
If you look at the original commit that introduced this test (8ec27a7), it was actually a non-existent server rather than a server that returned 500. Maybe both tests have value? |
The naming of the replication test which accessed the "down server" implies it's attempting to connect to a server which is down.
That test previously passed for any non-pouchdb server, including:
This commit:
Ref: #8656 #8421