Skip to content

Commit 69b8fa7

Browse files
SwatinemSwatinem
authored
Remove the get_changes_using_rust fn (#584)
This function was called within worker in "comparison mode", that ran both rust and python implementations side by side, only to always prefer the python results, and spam the logs with differences between the two that were never resolved, and I doubt anyone has looked at those in recent times. So lets just remove this code so there is less to maintain and to worry about going forward.
1 parent bdafe67 commit 69b8fa7

File tree

2 files changed

+1
-373
lines changed

2 files changed

+1
-373
lines changed

‎shared/reports/changes.py

-46
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,9 @@
11
import cc_rustyribs
22

3-
from shared.reports.types import Change
4-
53

64
def run_comparison_using_rust(base_report, head_report, diff):
75
return cc_rustyribs.run_comparison(
86
base_report.rust_report.get_report(),
97
head_report.rust_report.get_report(),
108
cc_rustyribs.rustify_diff(diff),
119
)
12-
13-
14-
def get_changes_using_rust(base_report, head_report, diff):
15-
return _get_changes_from_comparison(
16-
run_comparison_using_rust(base_report, head_report, diff)
17-
)
18-
19-
20-
def _get_changes_from_comparison(data):
21-
changes = []
22-
for found_change in data["files"]:
23-
if found_change["unexpected_line_changes"]:
24-
# temporary logic just to ensure we know all differences between
25-
# python changes and rust changes
26-
# on python, two partial lines are not considered unexpected changes
27-
# even if the fractions inside are different
28-
# in rust they are considered different, so we might see a few:
29-
# "unexpected_line_changes": [[[1, "p"], [1, "p"]]],
30-
# on rust
31-
has_actual_expected_line_changes = any(
32-
b[1] != h[1] for (b, h) in found_change["unexpected_line_changes"]
33-
)
34-
if has_actual_expected_line_changes:
35-
changes.append(
36-
Change(
37-
path=found_change["head_name"],
38-
in_diff=bool(found_change["added_diff_coverage"]),
39-
old_path=found_change.get("base_name")
40-
if found_change["base_name"] != found_change["head_name"]
41-
else None,
42-
totals=None,
43-
new=(
44-
found_change["head_coverage"] is not None
45-
and found_change["base_coverage"] is None
46-
and not found_change["file_was_added_by_diff"]
47-
),
48-
deleted=(
49-
found_change["base_coverage"] is not None
50-
and found_change["head_coverage"] is None
51-
and not found_change["file_was_removed_by_diff"]
52-
),
53-
)
54-
)
55-
return changes

‎tests/unit/reports/test_changes.py

+1-327
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,8 @@
33
import pytest
44
from cc_rustyribs import rustify_diff
55

6-
from shared.reports.changes import (
7-
_get_changes_from_comparison,
8-
get_changes_using_rust,
9-
run_comparison_using_rust,
10-
)
6+
from shared.reports.changes import run_comparison_using_rust
117
from shared.reports.readonly import ReadOnlyReport
12-
from shared.reports.types import Change
138

149
current_file = Path(__file__)
1510

@@ -128,327 +123,6 @@ def test_run_comparison_using_rust(sample_rust_report):
128123
}
129124

130125

131-
def test_get_changes_using_rust(sample_rust_report):
132-
base_report, head_report = sample_rust_report, sample_rust_report
133-
diff = {
134-
"files": {
135-
"tests/__init__.py": {
136-
"type": "modified",
137-
"before": None,
138-
"segments": [
139-
{
140-
"header": ["1", "3", "1", "5"],
141-
"lines": [
142-
"+sudo: false",
143-
"+",
144-
" language: python",
145-
" ",
146-
" python:",
147-
],
148-
}
149-
],
150-
"stats": {"added": 2, "removed": 0},
151-
}
152-
}
153-
}
154-
k = get_changes_using_rust(base_report, head_report, diff)
155-
assert k == [
156-
Change(
157-
path="tests/__init__.py",
158-
new=False,
159-
deleted=False,
160-
in_diff=True,
161-
old_path=None,
162-
totals=None,
163-
)
164-
]
165-
166-
167-
def test_get_changes_from_comparison(mocker):
168-
res = _get_changes_from_comparison(
169-
{
170-
"files": [
171-
{
172-
"base_name": "ProgressBar/index.tsx",
173-
"head_name": "ProgressBar/index.tsx",
174-
"file_was_added_by_diff": False,
175-
"file_was_removed_by_diff": False,
176-
"base_coverage": {
177-
"hits": 29,
178-
"misses": 0,
179-
"partials": 0,
180-
"branches": 4,
181-
"sessions": 0,
182-
"complexity": 0,
183-
"complexity_total": 0,
184-
"methods": 4,
185-
},
186-
"head_coverage": {
187-
"hits": 29,
188-
"misses": 0,
189-
"partials": 0,
190-
"branches": 4,
191-
"sessions": 0,
192-
"complexity": 0,
193-
"complexity_total": 0,
194-
"methods": 4,
195-
},
196-
"removed_diff_coverage": [[8, "h"]],
197-
"added_diff_coverage": [[8, "h"]],
198-
"unexpected_line_changes": [],
199-
},
200-
{
201-
"base_name": "ProgressBar/ui.model.ts",
202-
"head_name": "ProgressBar/data.qa.ts",
203-
"file_was_added_by_diff": False,
204-
"file_was_removed_by_diff": False,
205-
"base_coverage": None,
206-
"head_coverage": {
207-
"hits": 2,
208-
"misses": 0,
209-
"partials": 0,
210-
"branches": 1,
211-
"sessions": 0,
212-
"complexity": 0,
213-
"complexity_total": 0,
214-
"methods": 0,
215-
},
216-
"removed_diff_coverage": None,
217-
"added_diff_coverage": [],
218-
"unexpected_line_changes": [
219-
[[1, None], [1, "h"]],
220-
[[2, None], [2, "h"]],
221-
],
222-
},
223-
{
224-
"base_name": "Loading/ui.model.ts",
225-
"head_name": "Loading/data.qa.ts",
226-
"file_was_added_by_diff": False,
227-
"file_was_removed_by_diff": False,
228-
"base_coverage": None,
229-
"head_coverage": {
230-
"hits": 3,
231-
"misses": 0,
232-
"partials": 0,
233-
"branches": 1,
234-
"sessions": 0,
235-
"complexity": 0,
236-
"complexity_total": 0,
237-
"methods": 0,
238-
},
239-
"removed_diff_coverage": None,
240-
"added_diff_coverage": [],
241-
"unexpected_line_changes": [
242-
[[1, None], [1, "h"]],
243-
[[2, None], [2, "h"]],
244-
[[3, None], [3, "h"]],
245-
],
246-
},
247-
{
248-
"base_name": "Complete/data.qa.ts",
249-
"head_name": "Complete/data.qa.ts",
250-
"file_was_added_by_diff": True,
251-
"file_was_removed_by_diff": False,
252-
"base_coverage": None,
253-
"head_coverage": {
254-
"hits": 4,
255-
"misses": 0,
256-
"partials": 0,
257-
"branches": 1,
258-
"sessions": 0,
259-
"complexity": 0,
260-
"complexity_total": 0,
261-
"methods": 0,
262-
},
263-
"removed_diff_coverage": None,
264-
"added_diff_coverage": [[1, "h"], [2, "h"], [3, "h"], [4, "h"]],
265-
"unexpected_line_changes": [],
266-
},
267-
{
268-
"base_name": "src/static/images/rewards/coin-drop.svg",
269-
"head_name": "src/static/images/rewards/coin-drop.svg",
270-
"file_was_added_by_diff": True,
271-
"file_was_removed_by_diff": False,
272-
"base_coverage": None,
273-
"head_coverage": {
274-
"hits": 1,
275-
"misses": 0,
276-
"partials": 0,
277-
"branches": 0,
278-
"sessions": 0,
279-
"complexity": 0,
280-
"complexity_total": 0,
281-
"methods": 0,
282-
},
283-
"removed_diff_coverage": None,
284-
"added_diff_coverage": [],
285-
"unexpected_line_changes": [],
286-
},
287-
{
288-
"base_name": "src/product/Rewards/pages/Splash/index.tsx",
289-
"head_name": "src/product/Rewards/pages/Splash/index.tsx",
290-
"file_was_added_by_diff": False,
291-
"file_was_removed_by_diff": False,
292-
"base_coverage": {
293-
"hits": 35,
294-
"misses": 14,
295-
"partials": 0,
296-
"branches": 9,
297-
"sessions": 0,
298-
"complexity": 0,
299-
"complexity_total": 0,
300-
"methods": 7,
301-
},
302-
"head_coverage": {
303-
"hits": 35,
304-
"misses": 14,
305-
"partials": 0,
306-
"branches": 9,
307-
"sessions": 0,
308-
"complexity": 0,
309-
"complexity_total": 0,
310-
"methods": 7,
311-
},
312-
"removed_diff_coverage": [[66, "m"]],
313-
"added_diff_coverage": [[66, "m"]],
314-
"unexpected_line_changes": [],
315-
},
316-
{
317-
"base_name": "Loading/index.tsx",
318-
"head_name": "Loading/index.tsx",
319-
"file_was_added_by_diff": False,
320-
"file_was_removed_by_diff": False,
321-
"base_coverage": {
322-
"hits": 49,
323-
"misses": 0,
324-
"partials": 0,
325-
"branches": 4,
326-
"sessions": 0,
327-
"complexity": 0,
328-
"complexity_total": 0,
329-
"methods": 10,
330-
},
331-
"head_coverage": {
332-
"hits": 49,
333-
"misses": 0,
334-
"partials": 0,
335-
"branches": 4,
336-
"sessions": 0,
337-
"complexity": 0,
338-
"complexity_total": 0,
339-
"methods": 10,
340-
},
341-
"removed_diff_coverage": [[17, "h"]],
342-
"added_diff_coverage": [[17, "h"]],
343-
"unexpected_line_changes": [],
344-
},
345-
{
346-
"base_name": "Complete/index.tsx",
347-
"head_name": "Complete/index.tsx",
348-
"file_was_added_by_diff": False,
349-
"file_was_removed_by_diff": False,
350-
"base_coverage": None,
351-
"head_coverage": {
352-
"hits": 25,
353-
"misses": 0,
354-
"partials": 0,
355-
"branches": 2,
356-
"sessions": 0,
357-
"complexity": 0,
358-
"complexity_total": 0,
359-
"methods": 4,
360-
},
361-
"removed_diff_coverage": None,
362-
"added_diff_coverage": [
363-
[2, "h"],
364-
[3, "h"],
365-
[4, "h"],
366-
[6, "h"],
367-
[7, "h"],
368-
[9, "h"],
369-
[11, "h"],
370-
[14, "h"],
371-
[17, "h"],
372-
[18, "h"],
373-
[36, "h"],
374-
[37, "h"],
375-
[41, "h"],
376-
[46, "h"],
377-
],
378-
"unexpected_line_changes": [
379-
[[1, None], [1, "h"]],
380-
[[4, None], [8, "h"]],
381-
[[6, None], [13, "h"]],
382-
[[9, None], [16, "h"]],
383-
[[11, None], [20, "h"]],
384-
[[12, None], [21, "h"]],
385-
[[13, None], [22, "h"]],
386-
[[19, None], [29, "h"]],
387-
[[20, None], [30, "h"]],
388-
[[25, None], [35, "h"]],
389-
[[30, None], [48, "h"]],
390-
],
391-
},
392-
],
393-
"changes_summary": {
394-
"patch_totals": {
395-
"hits": 20,
396-
"misses": 1,
397-
"partials": 0,
398-
"coverage": 0.95238096,
399-
}
400-
},
401-
}
402-
)
403-
expected_res = [
404-
"Complete/index.tsx",
405-
"Loading/data.qa.ts",
406-
"ProgressBar/data.qa.ts",
407-
]
408-
assert sorted(x.path for x in res) == expected_res
409-
410-
411-
def test_get_changes_from_comparison_same_line_type():
412-
res = _get_changes_from_comparison(
413-
{
414-
"files": [
415-
{
416-
"base_name": "base_name.ts",
417-
"head_name": "head_name.ts",
418-
"file_was_added_by_diff": False,
419-
"file_was_removed_by_diff": False,
420-
"base_coverage": None,
421-
"head_coverage": {
422-
"hits": 2,
423-
"misses": 0,
424-
"partials": 0,
425-
"branches": 1,
426-
"sessions": 0,
427-
"complexity": 0,
428-
"complexity_total": 0,
429-
"methods": 0,
430-
},
431-
"removed_diff_coverage": [[8, "h"]],
432-
"added_diff_coverage": [[8, "h"]],
433-
"unexpected_line_changes": [
434-
[[1, "p"], [1, "p"]],
435-
[[2, "p"], [2, "p"]],
436-
],
437-
}
438-
],
439-
"changes_summary": {
440-
"patch_totals": {
441-
"hits": 20,
442-
"misses": 1,
443-
"partials": 0,
444-
"coverage": 0.95238096,
445-
}
446-
},
447-
}
448-
)
449-
assert res == []
450-
451-
452126
class TestRustifyDiff(object):
453127
def test_rustify_diff_empty(self):
454128
assert rustify_diff({}) == {}

0 commit comments

Comments
 (0)