Skip to content

Commit 803cba7

Browse files
jimolearyjimoleary
committed
PR feedback.
1 parent 0f19b9d commit 803cba7

File tree

9 files changed

+147
-54
lines changed

9 files changed

+147
-54
lines changed

‎README.md

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ regexs for files etc.).
9797
"source_file_seen_count" : 1
9898
}
9999
// Aggregate task_mappings and task_mappings_tasks into the shape exported by the API.
100-
> db.task_mappings.aggregate({$match:{"source_file" : "src/mongo/executor/network_interface_mock.cpp"}}, {$lookup:{"from": "task_mappings_tasks", "localField": "source_file", "foreignField": "source_file", "as": "tasks" }}, {$project:{_id:0, 'tasks._id':0, 'tasks.source_file':0}}).pretty()
100+
> db.task_mappings.aggregate({$match:{"source_file" : "src/mongo/executor/network_interface_mock.cpp"}}, {$lookup:{"from": "task_mappings_tasks", "localField": "_id", "foreignField": "task_mapping_id", "as": "tasks" }}, {$project:{_id:0, 'tasks._id':0, 'tasks.task_mapping_id':0}}).pretty()
101101
{
102102
"source_file" : "src/mongo/executor/network_interface_mock.cpp",
103103
"branch" : "master",
@@ -121,18 +121,11 @@ regexs for files etc.).
121121
* _task_mappings_tasks_: The current task mappings tasks. Documents from this collection
122122
are joined to the _task_mappings_ documents _tasks_ field through the source_file field.
123123
```
124-
> db.task_mappings_tasks.find({"source_file" : "src/mongo/executor/network_interface_mock.cpp"}).pretty()
124+
> db.task_mappings_tasks.findOne()
125125
{
126126
"_id" : ...,
127127
"name" : "replica_sets_large_txns_format",
128-
"source_file" : "src/mongo/executor/network_interface_mock.cpp",
129-
"variant" : "enterprise-rhel-62-64-bit",
130-
"flip_count" : 1
131-
}
132-
{
133-
"_id" : ...,
134-
"name" : "replica_sets_large_txns_format_05_enterprise-rhel-62-64-bit",
135-
"source_file" : "src/mongo/executor/network_interface_mock.cpp",
128+
"task_mapping_id" : ...,
136129
"variant" : "enterprise-rhel-62-64-bit",
137130
"flip_count" : 1
138131
}
@@ -166,7 +159,7 @@ are joined to the _task_mappings_ documents _tasks_ field through the source_fil
166159
"source_file_seen_count" : 2
167160
}
168161
// Aggregate test_mappings and test_mappings_test_files into the shape exported by the API.
169-
> db.test_mappings.aggregate({$match:{"source_file" : "src/mongo/db/s/config/configsvr_create_database_command.cpp"}}, {$lookup:{"from": "test_mappings_test_files", "localField": "source_file", "foreignField": "source_file", "as": "test_files" }}, {$project:{_id:0, 'test_files._id':0, 'test_files.source_file':0}}).pretty()
162+
> db.test_mappings.aggregate({$match:{"source_file" : "src/mongo/db/s/config/configsvr_create_database_command.cpp"}}, {$lookup:{"from": "test_mappings_test_files", "localField": "_id", "foreignField": "test_mapping_id", "as": "test_files" }}, {$project:{_id:0, 'test_files._id':0, 'test_files.test_mapping_id':0}}).pretty()
170163
{
171164
"source_file" : "src/mongo/db/s/config/configsvr_create_database_command.cpp",
172165
"branch" : "master",
@@ -189,7 +182,7 @@ are joined to _test_mappings_ documents _test_files_ field through the source_f
189182
{
190183
"_id" : ...,
191184
"name" : "jstests/core/views/views_validation.js",
192-
"source_file" : "src/mongo/db/s/config/configsvr_create_database_command.cpp",
185+
"test_mpapping_id" : ...,
193186
"test_file_seen_count" : 2
194187
}
195188
```

‎src/selectedtests/datasource/datasource_cli.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ def setup_mappings_tasks_indexes(collection: Collection) -> None:
5353
5454
:param collection: Collection to add indexes to.
5555
"""
56-
# task_mapping_id simplifies the index (but then requires a find_and_update_one for the
57-
# insert / upsert.
5856
index = IndexModel(
5957
[("task_mapping_id", ASCENDING), ("name", ASCENDING), ("variant", ASCENDING)], unique=True
6058
)
@@ -70,8 +68,6 @@ def setup_mappings_test_files_indexes(collection: Collection) -> None:
7068
7169
:param collection: Collection to add indexes to.
7270
"""
73-
# test_mapping_id simplifies the index (but then requires a find_and_update_one for the
74-
# insert / upsert.
7571
index = IndexModel([("test_mapping_id", ASCENDING), ("name", ASCENDING)], unique=True)
7672
collection.create_indexes([index])
7773
LOGGER.info("Adding indexes for collection", collection=collection.name)

‎src/selectedtests/helpers.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ def get_mongo_wrapper() -> MongoWrapper:
3333

3434

3535
def create_query(
36-
document, mutable: Optional[List[str]] = None, joined: Optional[List[str]] = None
36+
document: Dict[str, Any],
37+
mutable: Optional[List[str]] = None,
38+
joined: Optional[List[str]] = None,
3739
) -> Dict[str, Any]:
3840
"""
3941
Create a query document by shallow copying document and excluding the mutable and optional keys.

‎src/selectedtests/task_mappings/get_task_mappings.py

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,25 +37,20 @@ def get_correlated_task_mappings(
3737
"input": "$tasks",
3838
"as": "task",
3939
"cond": {
40-
"$and": [
40+
"$gte": [
4141
{
42-
"$gte": [
43-
{
44-
"$divide": [
45-
"$$task.flip_count",
46-
"$source_file_seen_count",
47-
]
48-
},
49-
float(threshold),
42+
"$divide": [
43+
"$$task.flip_count",
44+
"$source_file_seen_count",
5045
]
51-
}
46+
},
47+
float(threshold),
5248
]
5349
},
5450
}
5551
}
5652
}
5753
},
58-
# clean up the output before returning.
5954
{"$project": {"_id": False, "tasks._id": False, "tasks.task_mapping_id": False}},
6055
]
6156
)

‎src/selectedtests/test_mappings/update_test_mappings.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ def update_test_mappings_test_files(
5151

5252
def update_test_mappings(test_mappings: List[Dict[str, Any]], mongo: MongoWrapper) -> None:
5353
"""
54-
Update test mappings in the test mappings project config collection.
54+
Update test mappings in the test mappings collection.
5555
5656
:param test_mappings: A list of test mappings.
5757
:param mongo: An instance of MongoWrapper.

‎tests/selectedtests/task_mappings/test_update_task_mappings.py

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
from unittest.mock import MagicMock, patch
22

3+
import pytest
4+
35
from pymongo import ReturnDocument
6+
from pymongo.errors import BulkWriteError
47

58
import selectedtests.task_mappings.update_task_mappings as under_test
69

@@ -68,8 +71,8 @@ def test_task_mappings_are_updated(
6871

6972

7073
class TestUpdateTaskMappings:
71-
@patch(ns("UpdateOne"), autospec=True)
72-
def test_task_mappings_are_updated(self, update_one_mock):
74+
@patch(ns("update_task_mappings_tasks"), autospec=True)
75+
def test_task_mappings_are_updated(self, update_task_mappings_tasks_mock):
7376
mongo_mock = MagicMock()
7477

7578
source_file_seen_count = 1
@@ -98,12 +101,58 @@ def test_task_mappings_are_updated(self, update_one_mock):
98101
upsert=True,
99102
return_document=ReturnDocument.AFTER,
100103
)
104+
update_task_mappings_tasks_mock.assert_called_once_with(
105+
[task], {"task_mapping_id": 1}, mongo_mock
106+
)
107+
108+
109+
class TestUpdateTaskMappingsTasks:
110+
@patch(ns("UpdateOne"), autospec=True)
111+
def test_task_mappings_are_updated(self, update_one_mock):
112+
mongo_mock = MagicMock()
113+
114+
task_mapping_id = {"task_mapping_id": 1}
115+
task = {
116+
"name": "query_fuzzer_standalone_3_enterprise-rhel-62-64-bit",
117+
"variant": "enterprise-rhel-62-64-bit",
118+
"flip_count": 1,
119+
}
101120

121+
under_test.update_task_mappings_tasks([task], task_mapping_id, mongo_mock)
102122
update_one_mock.assert_called_once_with(
103123
{"name": task["name"], "variant": task["variant"], "task_mapping_id": 1},
104124
{"$inc": {"flip_count": task["flip_count"]}},
105125
upsert=True,
106126
)
127+
107128
mongo_mock.task_mappings_tasks.return_value.bulk_write.assert_called_once_with(
108129
[update_one_mock.return_value]
109130
)
131+
132+
@patch(ns("LOGGER.exception"), autospec=True)
133+
@patch(ns("UpdateOne"), autospec=True)
134+
def test_task_mappings_exceptions(self, update_one_mock, exception_mock):
135+
mongo_mock = MagicMock()
136+
137+
task_mapping_id = {"task_mapping_id": 1}
138+
task = {
139+
"name": "query_fuzzer_standalone_3_enterprise-rhel-62-64-bit",
140+
"variant": "enterprise-rhel-62-64-bit",
141+
"flip_count": 1,
142+
}
143+
144+
details = {"errorLabels": []}
145+
mongo_mock.task_mappings_tasks.return_value.bulk_write.side_effect = BulkWriteError(details)
146+
pytest.raises(
147+
BulkWriteError,
148+
under_test.update_task_mappings_tasks,
149+
[task],
150+
task_mapping_id,
151+
mongo_mock,
152+
)
153+
exception_mock.assert_called_once_with(
154+
"bulk_write error",
155+
parent=task_mapping_id,
156+
operations=[update_one_mock.return_value],
157+
details=details,
158+
)

‎tests/selectedtests/test_create_query.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import selectedtests.helpers as under_test
2+
3+
NS = "selectedtests.evergreen_helper"
4+
5+
6+
def ns(relative_name):
7+
"""Return a full name from a name relative to the tested module"s name space."""
8+
return NS + "." + relative_name
9+
10+
11+
class TestCreateQuery:
12+
def test_no_mutable_joined(self):
13+
query = under_test.create_query({"imput": 1})
14+
assert query == {"imput": 1}
15+
16+
def test_mutable_and_joined(self):
17+
query = under_test.create_query(
18+
{"imput": 1, "flip_count": 2, "tests": 3},
19+
mutable=["flip_count", "source_file_seen_count"],
20+
joined=["tests", "tasks"],
21+
)
22+
assert query == {"imput": 1}

‎tests/selectedtests/test_mappings/test_get_test_mappings.py

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,30 +7,16 @@ class TestGetCorrelatedTestMappings:
77
def test_mappings_found(self):
88
collection_mock = MagicMock()
99
threshold = 0.5
10-
test_mappings = [
11-
{
12-
"project": "my_project",
13-
"source_file": "src/file1.js",
14-
"source_file_seen_count": 3,
15-
"test_files": [
16-
{"name": "test_file_above_threshold.js", "test_file_seen_count": 2},
17-
{"name": "test_file_below_threshold.js", "test_file_seen_count": 1},
18-
],
19-
},
20-
{
21-
"project": "my_project",
22-
"source_file": "src/file2.js",
23-
"source_file_seen_count": 1,
24-
"test_files": [{"name": "test1.js", "test_file_seen_count": 1}],
25-
},
26-
]
27-
collection_mock.aggregate.side_effect = test_mappings
10+
11+
collection_mock.aggregate.return_value = ["results"]
2812
changed_files = ["src/file1.js", "src/file2.js"]
2913
project = "my-project"
30-
under_test.get_correlated_test_mappings(collection_mock, changed_files, project, threshold)
14+
test_mappings = under_test.get_correlated_test_mappings(
15+
collection_mock, changed_files, project, threshold
16+
)
3117

3218
collection_mock.aggregate.assert_called_once()
33-
assert test_mappings == test_mappings
19+
assert ["results"] == test_mappings
3420

3521
def test_no_mappings_found(self):
3622
collection_mock = MagicMock()

‎tests/selectedtests/test_mappings/test_update_test_mappings.py

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
from unittest.mock import MagicMock, patch
22

3+
import pytest
4+
35
from pymongo import ReturnDocument
6+
from pymongo.errors import BulkWriteError
47

58
import selectedtests.test_mappings.update_test_mappings as under_test
69

@@ -74,8 +77,8 @@ def test_mappings_are_updated(
7477

7578

7679
class TestUpdateTestMappings:
77-
@patch(ns("UpdateOne"), autospec=True)
78-
def test_mappings_are_updated(self, update_one_mock):
80+
@patch(ns("update_test_mappings_test_files"), autospec=True)
81+
def test_mappings_are_updated(self, update_test_mappings_test_files_mock):
7982
mongo_mock = MagicMock()
8083

8184
source_file_seen_count = 1
@@ -107,11 +110,58 @@ def test_mappings_are_updated(self, update_one_mock):
107110
return_document=ReturnDocument.AFTER,
108111
)
109112

113+
update_test_mappings_test_files_mock.assert_called_once_with(
114+
[test_file], {"test_mapping_id": 1}, mongo_mock
115+
)
116+
117+
118+
class TestUpdateTestMappingsTestFiles:
119+
@patch(ns("UpdateOne"), autospec=True)
120+
def test_test_mappings_test_files_are_updated(self, update_one_mock):
121+
mongo_mock = MagicMock()
122+
123+
test_mapping_id = {"test_mapping_id": 1}
124+
test_file = {
125+
"name": "jstests/core/txns/commands_not_allowed_in_txn.js",
126+
"test_file_seen_count": 1,
127+
}
128+
129+
under_test.update_test_mappings_test_files([test_file], test_mapping_id, mongo_mock)
110130
update_one_mock.assert_called_once_with(
111-
{"test_mapping_id": 1, "name": test_file["name"]},
131+
{"name": test_file["name"], "test_mapping_id": 1},
112132
{"$inc": {"test_file_seen_count": test_file["test_file_seen_count"]}},
113133
upsert=True,
114134
)
135+
115136
mongo_mock.test_mappings_test_files.return_value.bulk_write.assert_called_once_with(
116137
[update_one_mock.return_value]
117138
)
139+
140+
@patch(ns("LOGGER.exception"), autospec=True)
141+
@patch(ns("UpdateOne"), autospec=True)
142+
def test_task_mappings_exceptions(self, update_one_mock, exception_mock):
143+
mongo_mock = MagicMock()
144+
145+
test_mapping_id = {"test_mapping_id": 1}
146+
test_file = {
147+
"name": "jstests/core/txns/commands_not_allowed_in_txn.js",
148+
"test_file_seen_count": 1,
149+
}
150+
151+
details = {"errorLabels": []}
152+
mongo_mock.test_mappings_test_files.return_value.bulk_write.side_effect = BulkWriteError(
153+
details
154+
)
155+
pytest.raises(
156+
BulkWriteError,
157+
under_test.update_test_mappings_test_files,
158+
[test_file],
159+
test_mapping_id,
160+
mongo_mock,
161+
)
162+
exception_mock.assert_called_once_with(
163+
"bulk_write error",
164+
parent=test_mapping_id,
165+
operations=[update_one_mock.return_value],
166+
details=details,
167+
)

0 commit comments

Comments
 (0)