fix tests for mongodb 3.4 #188

saghm · 2017-02-22T02:54:38Z

Two tests were failing when run against 3.4, both due to the order of things being returned having changed. Along with fixing them, I added 3.4 testing to the travis config.

saghm · 2017-02-22T02:55:54Z

.travis.yml


 script:
-    - mkdir -p ./data/db30 ./data/db30-ssl ./data/db32 ./data/db32-ssl
+    - mkdir -p ./data/db30 ./data/db30-ssl ./data/db32 ./data/db32-ssl ./data/db34 ./data/db34-ssl


This is starting to get a bit messy; we should try to look into using mongo orchestration or some other method of managing testing against the different server versions, ideally before 3.6 comes out and we have to add that as well.

Added a tracking issue for Morc (this is a thing now. i'm making this name happen)

saghm · 2017-02-22T02:56:35Z

tests/apm/mod.rs

@@ -54,7 +54,7 @@ fn read_first_non_monitor_line(file: &mut BufReader<&File>, line: &mut String) {

 #[test]
 fn logging() {
-    let _ = fs::remove_file("test_apm_log.txt");
+    let _ = fs::remove_file("test_log.txt");


For some reason the old name of this file didn't match the name I put in the .gitignore way back when this test was created, so I fixed that.

That might have been my fault -- i changed a lot of stringy fields to be more specific when I was parallelizing all of the tests.

saghm · 2017-02-22T02:59:12Z

tests/apm/mod.rs

@@ -170,5 +180,5 @@ fn logging() {

    coll.drop().unwrap();

-    fs::remove_file("test_apm_log.txt").unwrap();
+    fs::remove_file("test_log.txt").unwrap();


I've noticed/started to get annoyed that the test file doesn't get cleaned up when the tests fail above this line, so I'm thinking we should change the tests so this file doesn't clutter things up. Given that the file should only be about a dozen lines, I think reading the whole thing into a string and then iterating over the lines should be fine.

Makes sense to me for now. In an ideal future we would allow the client to take in a generic Write-implementing object and pass in a string-like IO object for testing purposes; if with_log_file(str) is part of the spec, it can be added as a utility method.

That's a good idea; it would be much more idiomatic that way.

saghm · 2017-02-22T02:59:55Z

tests/client/db.rs

@@ -35,7 +35,7 @@ fn create_collection() {

    let db1 = if v3_1 { "test1" } else { "test2" };
    let db2 = if v3_1 { "test2" } else { "test1" };
-    
+


See, rustfmt does actually help sometimes; empty lines shouldn't have extra whitespace

a rare win, but still on thin ice. i'm watching you, rustfmt

saghm · 2017-02-22T03:01:46Z

tests/client/db.rs

@@ -101,6 +101,17 @@ fn create_and_get_users() {
    db.drop_database().unwrap();
    db.drop_all_users(None).unwrap();

+    let kevin_options = CreateUserOptions {


Given that versions below 3.4 return the users in the order they're created and 3.4 returns them in lexicographical order, the easiest fix for this is to just create the users in the order where all versions expect the same thing.

kyeah

cool, looks good for merging once tests are passing on Travis.

kyeah · 2017-02-22T04:37:13Z

tests/apm/mod.rs

@@ -54,7 +54,7 @@ fn read_first_non_monitor_line(file: &mut BufReader<&File>, line: &mut String) {

 #[test]
 fn logging() {
-    let _ = fs::remove_file("test_apm_log.txt");
+    let _ = fs::remove_file("test_log.txt");


That might have been my fault -- i changed a lot of stringy fields to be more specific when I was parallelizing all of the tests.

kyeah · 2017-02-22T04:42:39Z

tests/apm/mod.rs

+    let insert_one_line_start = if v3_3 {
+        "COMMAND.insert_one 127.0.0.1:27017 COMPLETED: { ok: 1, n: 1 } ("
+    } else {
+        "COMMAND.insert_one 127.0.0.1:27017 COMPLETED: { n: 1, ok: 1 } ("


argh, drives me crazy. this is the expected apm behavior for drivers, yeah? to print it out as ordered by the server, rather than always be lexicographic?

I'll double check, but I'd guess that the spec doesn't even mention it

yeah, makes sense as long as we're supposed to print document fields in insert order (most likely) and read server responses in insert order (probably doesn't even mention this in any of the documentation, but we wouldn't want to do any extra work here to alphabetize anyways).

kyeah · 2017-02-22T04:47:25Z

tests/apm/mod.rs

@@ -170,5 +180,5 @@ fn logging() {

    coll.drop().unwrap();

-    fs::remove_file("test_apm_log.txt").unwrap();
+    fs::remove_file("test_log.txt").unwrap();


Makes sense to me for now. In an ideal future we would allow the client to take in a generic Write-implementing object and pass in a string-like IO object for testing purposes; if with_log_file(str) is part of the spec, it can be added as a utility method.

kyeah · 2017-02-22T04:48:46Z

tests/client/db.rs

@@ -35,7 +35,7 @@ fn create_collection() {

    let db1 = if v3_1 { "test1" } else { "test2" };
    let db2 = if v3_1 { "test2" } else { "test1" };
-    
+


a rare win, but still on thin ice. i'm watching you, rustfmt

kyeah · 2017-02-22T04:50:10Z

tests/client/db.rs

@@ -101,6 +101,17 @@ fn create_and_get_users() {
    db.drop_database().unwrap();
    db.drop_all_users(None).unwrap();

+    let kevin_options = CreateUserOptions {


kyeah · 2017-02-22T04:50:25Z

tests/client/db.rs

+    };
+
+    db.create_user("kevin",
+                     "ihavenosenseofhumorandthereforeihatepuns!",


Haha, I had forgotten about that until I made this change, but I'm not even slightly surprised that did this

saghm · 2017-02-22T17:13:48Z

I'm a bit confused about why Github shows Travis as "not yet completed" when the task clearly finished last night...

fix tests for mongodb 3.4

80938fb

saghm assigned kyeah Feb 22, 2017

saghm commented Feb 22, 2017

View reviewed changes

kyeah mentioned this pull request Feb 22, 2017

Use mongo-orchestration for testing #189

Closed

kyeah reviewed Feb 22, 2017

View reviewed changes

kyeah approved these changes Feb 22, 2017

View reviewed changes

saghm merged commit f1967c9 into mongodb-labs:master Feb 22, 2017

saghm mentioned this pull request Feb 23, 2017

3.4 compatibility #179

Closed

saghm deleted the fix-34-tests branch April 14, 2017 05:29

		@@ -35,7 +35,7 @@ fn create_collection() {

		let db1 = if v3_1 { "test1" } else { "test2" };
		let db2 = if v3_1 { "test2" } else { "test1" };

fix tests for mongodb 3.4 #188

fix tests for mongodb 3.4 #188

Uh oh!

Conversation

saghm commented Feb 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyeah Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyeah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyeah Feb 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saghm commented Feb 22, 2017

Uh oh!

Uh oh!

kyeah Feb 22, 2017 •

edited

Loading

kyeah Feb 22, 2017 •

edited

Loading