Skip to content
This repository was archived by the owner on Oct 18, 2021. It is now read-only.

fix tests for mongodb 3.4 #188

Merged
merged 1 commit into from
Feb 22, 2017
Merged

fix tests for mongodb 3.4 #188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,34 @@ before_install:
- wget http://fastdl.mongodb.org/linux/mongodb-linux-x86_64-ubuntu1204-3.0.14.tgz
- tar xvf mongodb-linux-x86_64-ubuntu1204-3.0.14.tgz
- mv mongodb-linux-x86_64-ubuntu1204-3.0.14 3.0.14
- wget http://fastdl.mongodb.org/linux/mongodb-linux-x86_64-ubuntu1204-3.2.11.tgz
- tar xvf mongodb-linux-x86_64-ubuntu1204-3.2.11.tgz
- mv mongodb-linux-x86_64-ubuntu1204-3.2.11 3.2.11
- wget http://fastdl.mongodb.org/linux/mongodb-linux-x86_64-ubuntu1204-3.2.12.tgz
- tar xvf mongodb-linux-x86_64-ubuntu1204-3.2.12.tgz
- mv mongodb-linux-x86_64-ubuntu1204-3.2.12 3.2.12
- wget http://fastdl.mongodb.org/linux/mongodb-linux-x86_64-ubuntu1204-3.4.2.tgz
- tar xvf mongodb-linux-x86_64-ubuntu1204-3.4.2.tgz
- mv mongodb-linux-x86_64-ubuntu1204-3.4.2 3.4.2

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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

- 3.0.14/bin/mongod --fork --dbpath ./data/db30 --syslog --port 27017
- cargo build --verbose
- cargo test --verbose
- killall mongod
- 3.2.11/bin/mongod --fork --dbpath ./data/db32 --syslog --port 27017
- 3.2.12/bin/mongod --fork --dbpath ./data/db32 --syslog --port 27017
- cargo test --verbose
- killall mongod
- 3.4.2/bin/mongod --fork --dbpath ./data/db34 --syslog --port 27017
- cargo test --verbose
- killall mongod
- 3.0.14/bin/mongod --fork --dbpath ./data/db30 --syslog --port 27017
- 3.0.14/bin/mongod --fork --dbpath ./data/db30-ssl --syslog --port 27018 --sslMode requireSSL --sslPEMKeyFile tests/ssl/server.pem --sslCAFile tests/ssl/ca.pem
- cargo build --features ssl --verbose
- cargo test --features ssl --verbose
- killall mongod
- 3.2.11/bin/mongod --fork --dbpath ./data/db32 --syslog --port 27017
- 3.2.11/bin/mongod --fork --dbpath ./data/db32-ssl --syslog --port 27018 --sslMode requireSSL --sslPEMKeyFile tests/ssl/server.pem --sslCAFile tests/ssl/ca.pem
- 3.2.12/bin/mongod --fork --dbpath ./data/db32 --syslog --port 27017
- 3.2.12/bin/mongod --fork --dbpath ./data/db32-ssl --syslog --port 27018 --sslMode requireSSL --sslPEMKeyFile tests/ssl/server.pem --sslCAFile tests/ssl/ca.pem
- cargo test --features ssl --verbose
- killall mongod
- 3.4.2/bin/mongod --fork --dbpath ./data/db34 --syslog --port 27017
- 3.4.2/bin/mongod --fork --dbpath ./data/db34-ssl --syslog --port 27018 --sslMode requireSSL --sslPEMKeyFile tests/ssl/server.pem --sslCAFile tests/ssl/ca.pem
- cargo test --features ssl --verbose


24 changes: 17 additions & 7 deletions tests/apm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.


// Reset State
let reset_client = Client::connect("localhost", 27017)
Expand All @@ -63,8 +63,11 @@ fn logging() {
let reset_coll = reset_db.collection("logging");
reset_coll.drop().unwrap();

let db_version = reset_db.version().unwrap();
let v3_3 = db_version.major <= 3 && db_version.minor <= 3;

// Start logging
let client_options = ClientOptions::with_log_file("test_apm_log.txt");
let client_options = ClientOptions::with_log_file("test_log.txt");
let client = Client::connect_with_options("localhost", 27017, client_options).unwrap();

let db = client.db("test-apm-mod");
Expand All @@ -86,7 +89,7 @@ fn logging() {

coll.find(Some(filter), None).unwrap();

let f = File::open("test_apm_log.txt").unwrap();
let f = File::open("test_log.txt").unwrap();
let mut file = BufReader::new(&f);
let mut line = String::new();

Expand Down Expand Up @@ -121,10 +124,17 @@ fn logging() {
_id: 1 }] }\n",
&line);


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 } ("
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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).

};

// First insert completed
line.clear();
read_first_non_monitor_line(&mut file, &mut line);
assert!(line.starts_with("COMMAND.insert_one 127.0.0.1:27017 COMPLETED: { ok: 1, n: 1 } ("));
assert!(line.starts_with(insert_one_line_start));
assert!(line.ends_with(" ns)\n"));

// Second insert started
Expand All @@ -137,7 +147,7 @@ fn logging() {
// Second insert completed
line.clear();
read_first_non_monitor_line(&mut file, &mut line);
assert!(line.starts_with("COMMAND.insert_one 127.0.0.1:27017 COMPLETED: { ok: 1, n: 1 } ("));
assert!(line.starts_with(insert_one_line_start));
assert!(line.ends_with(" ns)\n"));

// Third insert started
Expand All @@ -150,7 +160,7 @@ fn logging() {
// Third insert completed
line.clear();
read_first_non_monitor_line(&mut file, &mut line);
assert!(line.starts_with("COMMAND.insert_one 127.0.0.1:27017 COMPLETED: { ok: 1, n: 1 } ("));
assert!(line.starts_with(insert_one_line_start));
assert!(line.ends_with(" ns)\n"));

// Find command started
Expand All @@ -170,5 +180,5 @@ fn logging() {

coll.drop().unwrap();

fs::remove_file("test_apm_log.txt").unwrap();
fs::remove_file("test_log.txt").unwrap();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@kyeah kyeah Feb 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

}
33 changes: 17 additions & 16 deletions tests/client/db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn create_collection() {

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

match results[result_size - 2].get("name") {
Some(&Bson::String(ref name)) => assert_eq!(db1, name),
_ => panic!("Expected BSON string!"),
Expand Down Expand Up @@ -82,7 +82,7 @@ fn list_collections() {

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

match results[result_size - 2].get("name") {
Some(&Bson::String(ref name)) => assert_eq!(db1, name),
_ => panic!("Expected BSON string!"),
Expand All @@ -101,6 +101,17 @@ fn create_and_get_users() {
db.drop_database().unwrap();
db.drop_all_users(None).unwrap();

let kevin_options = CreateUserOptions {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice.

custom_data: None,
roles: vec![Role::All(AllDatabaseRole::Read)],
write_concern: None,
};

db.create_user("kevin",
"ihavenosenseofhumorandthereforeihatepuns!",
Copy link
Contributor

Choose a reason for hiding this comment

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

lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Some(kevin_options))
.unwrap();

let saghm_options = CreateUserOptions {
custom_data: Some(doc! { "foo" => "bar" }),
roles: vec![Role::Single {
Expand All @@ -113,16 +124,6 @@ fn create_and_get_users() {

db.create_user("saghm", "ilikepuns!", Some(saghm_options)).unwrap();

let kevin_options = CreateUserOptions {
custom_data: None,
roles: vec![Role::All(AllDatabaseRole::Read)],
write_concern: None,
};

db.create_user("kevin",
"ihavenosenseofhumorandthereforeihatepuns!",
Some(kevin_options))
.unwrap();
db.create_user("val", "ilikeangularjs!", None).unwrap();

let user = db.get_user("saghm", None).unwrap();
Expand Down Expand Up @@ -163,13 +164,13 @@ fn create_and_get_users() {
assert_eq!(users.len(), 3);

match users[0].get("user") {
Some(&Bson::String(ref s)) => assert_eq!("saghm", s),
_ => panic!("User isn't named 'saghm' but should be"),
Some(&Bson::String(ref s)) => assert_eq!("kevin", s),
_ => panic!("User isn't named 'kevin' but should be"),
};

match users[1].get("user") {
Some(&Bson::String(ref s)) => assert_eq!("kevin", s),
_ => panic!("User isn't named 'kevin' but should be"),
Some(&Bson::String(ref s)) => assert_eq!("saghm", s),
_ => panic!("User isn't named 'saghm' but should be"),
};

match users[2].get("user") {
Expand Down