Skip to content

PHPC-1015: Add test for DNS Initial Seedlist #685

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

Merged
merged 2 commits into from
Dec 5, 2017

Conversation

derickr
Copy link
Contributor

@derickr derickr commented Dec 1, 2017

https://jira.mongodb.org/browse/PHPC-1015

We'll need to decide whether we want to always run this. Starting up a whole
replicaset for one test is a waste of resources really. It also requires that we add a line in our /etc/hosts files (with the IP address of the Vagrant host) for this to work.

@derickr derickr requested a review from jmikola December 1, 2017 12:34
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

It also requires that we add a line in our /etc/hosts files (with the IP address of the Vagrant host) for this to work.

Is this required change documented anywhere? Perhaps it can be added in a block comment within the test?

Am I correct to assume that the test fails without the /etc/hosts change in place?

I think it's worth having the Vagrant environment committed. We can probably leave this as a manual test for the time being and revisit once we can move over to Evergreen. To that end, a separate tracking ticket associated with the Evergreen epic and also linked to PHPC-1015 would be helpful.

@@ -375,7 +375,11 @@ if test "$MONGODB" != "no"; then

m4_include(src/libmongoc/build/autotools/m4/ax_prototype.m4)
m4_include(src/libmongoc/build/autotools/CheckCompiler.m4)

dnl We need to convince the libmongoc M4 file to actually run these checks for us
enable_srv=auto
Copy link
Member

Choose a reason for hiding this comment

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

Interesting. Is this necessary because we do not include ReadCommandLineArguments.m4? AFAIK, this is one of the few times we have to set a variable this way (instead of using AC replacement).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup.

@derickr
Copy link
Contributor Author

derickr commented Dec 1, 2017

Is this required change documented anywhere? Perhaps it can be added in a block comment within the test?

No, but I can add that.

Am I correct to assume that the test fails without the /etc/hosts change in place?

Yes:

Fatal error: Uncaught MongoDB\Driver\Exception\ConnectionTimeoutException: No suitable servers found (`serverSelectionTryOnce` set): [connection refused calling ismaster on 'localhost.test.build.10gen.cc:27018'] [connection refused calling ismaster on 'localhost.test.build.10gen.cc:27017'] in /home/derick/dev/php/derickr-mongo-php-driver/tests/connect/bug1015.php:7
Stack trace:
#0 /home/derick/dev/php/derickr-mongo-php-driver/tests/connect/bug1015.php(7): MongoDB\Driver\Manager->selectServer(Object(MongoDB\Driver\ReadPreference))
#1 {main}
  thrown in /home/derick/dev/php/derickr-mongo-php-driver/tests/connect/bug1015.php on line 7

I think it's worth having the Vagrant environment committed. We can probably leave this as a manual test for the time being and revisit once we can move over to Evergreen. To that end, a separate tracking ticket associated with the Evergreen epic and also linked to PHPC-1015 would be helpful.

Not sure how easy this is going to be with Evergreen either, but: https://jira.mongodb.org/browse/PHPC-1054

We'll need to decide whether we want to always run this. Starting up a whole
replicaset for one test is a waste of resources really.
@derickr derickr force-pushed the PHPC-1015-dns-initial-seedlist branch from cddb418 to 15e1450 Compare December 5, 2017 10:36
@derickr derickr merged commit 15e1450 into mongodb:master Dec 5, 2017
derickr added a commit that referenced this pull request Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants