Skip to content

Commit cb3ef6e

Browse files
author
Snapshot Budgets
committed
Make the main tree pass the full quality contract
Register the enrollment accessibility and sorting test modules with the CI adoption gate and name them in the strict typecheck override; fix the tar-spec annotations and use the production limits dataclass in the snapshot resource budgets tests; format the three files the contract's format component flagged.
1 parent f2b32dd commit cb3ef6e

7 files changed

Lines changed: 41 additions & 58 deletions

File tree

content_sync/tests/test_snapshot_resource_budgets.py

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import subprocess
1919
import tarfile
2020
import time
21-
from dataclasses import dataclass
2221
from pathlib import Path
2322
from unittest.mock import patch
2423

@@ -27,6 +26,7 @@
2726
from content_sync import snapshot
2827
from content_sync.course_repository_ingest import (
2928
CourseRepositoryFetchError,
29+
CourseRepositoryLimits,
3030
fetch_course_repository_snapshot,
3131
read_course_repository_checkout,
3232
)
@@ -35,16 +35,7 @@
3535
SCRATCH_ROOT = PROJECT_ROOT / ".tmp" / "snapshot-resource-budgets"
3636

3737

38-
@dataclass(frozen=True)
39-
class _Limits:
40-
"""The tightest ceilings the budgets can be expressed against."""
41-
42-
max_files: int = 1
43-
max_total_bytes: int = 1
44-
max_file_bytes: int = 1
45-
46-
47-
def _build_tar(*specs: tuple[str, int, bytes, dict[str, str] | None]) -> bytes:
38+
def _build_tar(*specs: tuple[str, bytes, bytes, dict[str, str] | None]) -> bytes:
4839
"""One tar session holding every spec: ``(name, type, content, pax)``."""
4940

5041
buffer = io.BytesIO()
@@ -59,11 +50,11 @@ def _build_tar(*specs: tuple[str, int, bytes, dict[str, str] | None]) -> bytes:
5950
return buffer.getvalue()
6051

6152

62-
def _file(name: str, content: bytes = b"x") -> tuple[str, int, bytes, None]:
53+
def _file(name: str, content: bytes = b"x") -> tuple[str, bytes, bytes, None]:
6354
return (name, tarfile.REGTYPE, content, None)
6455

6556

66-
def _dir(name: str) -> tuple[str, int, bytes, None]:
57+
def _dir(name: str) -> tuple[str, bytes, bytes, None]:
6758
return (name, tarfile.DIRTYPE, b"", None)
6859

6960

@@ -147,7 +138,7 @@ def test_success_under_the_cap_returns_the_archive(self) -> None:
147138
self.assertEqual(
148139
snapshot.read_snapshot_archive(
149140
archive,
150-
limits=_Limits(max_files=5, max_total_bytes=100, max_file_bytes=100),
141+
limits=CourseRepositoryLimits(max_files=5, max_total_bytes=100, max_file_bytes=100),
151142
strip_root=False,
152143
),
153144
{"a.txt": b"hello\n"},
@@ -217,7 +208,11 @@ def test_many_directory_entries_exhaust_the_structural_budget(self) -> None:
217208
archive = _build_tar(*[_dir(f"dir-{index:03d}/") for index in range(100)])
218209

219210
with self.assertRaises(snapshot.SnapshotError) as raised:
220-
snapshot.read_snapshot_archive(archive, limits=_Limits(), strip_root=False)
211+
snapshot.read_snapshot_archive(
212+
archive,
213+
limits=CourseRepositoryLimits(max_files=1, max_total_bytes=1, max_file_bytes=1),
214+
strip_root=False,
215+
)
221216

222217
self.assertEqual(raised.exception.code, "archive_members_exceeded")
223218

@@ -227,7 +222,7 @@ def test_legitimate_directories_do_not_consume_the_file_count(self) -> None:
227222

228223
result = snapshot.read_snapshot_archive(
229224
_build_tar(*specs),
230-
limits=_Limits(max_files=10, max_total_bytes=100, max_file_bytes=10),
225+
limits=CourseRepositoryLimits(max_files=10, max_total_bytes=100, max_file_bytes=10),
231226
strip_root=False,
232227
)
233228

@@ -248,7 +243,9 @@ def test_a_compressed_metadata_bomb_is_bounded(self) -> None:
248243

249244
with self.assertRaises(snapshot.SnapshotError) as raised:
250245
snapshot.read_snapshot_archive(
251-
gzip.compress(_build_tar(*specs)), limits=_Limits(), strip_root=False
246+
gzip.compress(_build_tar(*specs)),
247+
limits=CourseRepositoryLimits(max_files=1, max_total_bytes=1, max_file_bytes=1),
248+
strip_root=False,
252249
)
253250

254251
self.assertEqual(raised.exception.code, "archive_expansion_exceeded")
@@ -258,7 +255,7 @@ def test_symlinks_and_duplicates_are_still_refused(self) -> None:
258255
with self.assertRaises(snapshot.SnapshotError) as raised:
259256
snapshot.read_snapshot_archive(
260257
_build_tar(link),
261-
limits=_Limits(max_files=5, max_total_bytes=100, max_file_bytes=10),
258+
limits=CourseRepositoryLimits(max_files=5, max_total_bytes=100, max_file_bytes=10),
262259
strip_root=False,
263260
)
264261
self.assertEqual(raised.exception.code, "archive_entry_invalid")
@@ -267,7 +264,7 @@ def test_symlinks_and_duplicates_are_still_refused(self) -> None:
267264
with self.assertRaises(snapshot.SnapshotError) as raised:
268265
snapshot.read_snapshot_archive(
269266
duplicate,
270-
limits=_Limits(max_files=5, max_total_bytes=100, max_file_bytes=10),
267+
limits=CourseRepositoryLimits(max_files=5, max_total_bytes=100, max_file_bytes=10),
271268
strip_root=False,
272269
)
273270
self.assertEqual(raised.exception.code, "duplicate_path")
@@ -325,7 +322,9 @@ def test_a_budget_spent_on_the_headers_refuses_before_reading(self) -> None:
325322
owner="owner",
326323
repository="repo",
327324
commit_sha="a" * 40,
328-
limits=_Limits(max_files=5, max_total_bytes=100, max_file_bytes=10),
325+
limits=CourseRepositoryLimits(
326+
max_files=5, max_total_bytes=100, max_file_bytes=10
327+
),
329328
)
330329

331330
self.assertEqual(raised.exception.code, "course_repository_fetch_timeout")
@@ -344,7 +343,9 @@ def test_a_budget_spent_midstream_stops_the_reads(self) -> None:
344343
owner="owner",
345344
repository="repo",
346345
commit_sha="a" * 40,
347-
limits=_Limits(max_files=5, max_total_bytes=100, max_file_bytes=10),
346+
limits=CourseRepositoryLimits(
347+
max_files=5, max_total_bytes=100, max_file_bytes=10
348+
),
348349
)
349350

350351
self.assertEqual(raised.exception.code, "course_repository_fetch_timeout")
@@ -358,7 +359,7 @@ class TransportBudgetParityTests(SimpleTestCase):
358359
def test_a_structurally_exhausting_tree_refuses_identically(self) -> None:
359360
deep = "/".join(f"level-{index:02d}" for index in range(100))
360361
root, commit_sha = _git_repo("deep-tree", file_relative=f"{deep}/deepest.txt")
361-
limits = _Limits(max_files=1, max_total_bytes=1_000_000, max_file_bytes=100)
362+
limits = CourseRepositoryLimits(max_files=1, max_total_bytes=1_000_000, max_file_bytes=100)
362363

363364
with self.assertRaises(CourseRepositoryFetchError) as pulled:
364365
read_course_repository_checkout(root, commit_sha=commit_sha, limits=limits)

events/tests/test_qna_lifecycle_contract.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,7 @@ def _poll(self) -> dict:
101101
def test_open_poll_reports_capabilities(self) -> None:
102102
payload = self._poll()
103103
self.assertEqual(payload["state"], "open")
104-
self.assertEqual(
105-
payload["capabilities"], {"can_ask": True, "can_vote": True}
106-
)
104+
self.assertEqual(payload["capabilities"], {"can_ask": True, "can_vote": True})
107105

108106
def test_closed_poll_answers_200_with_the_new_state(self) -> None:
109107
stale = self._poll()
@@ -115,9 +113,7 @@ def test_closed_poll_answers_200_with_the_new_state(self) -> None:
115113
self.assertEqual(response.status_code, 200)
116114
payload = response.json()
117115
self.assertEqual(payload["state"], "closed")
118-
self.assertEqual(
119-
payload["capabilities"], {"can_ask": False, "can_vote": False}
120-
)
116+
self.assertEqual(payload["capabilities"], {"can_ask": False, "can_vote": False})
121117

122118
def test_participant_page_renders_lifecycle_from_the_session(self) -> None:
123119
participant, _token = security.new_participant()

playwright_tests/test_enrollment_sorting.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,7 @@ def test_sort_is_global_and_survives_pagination_without_javascript(
163163
assert "page=" not in page.url
164164
settle(page, PAGE_SIZE)
165165
expect(first_row(page)).to_contain_text("page-student-01")
166-
expect(page.locator("th", has_text="Pos").first).to_have_attribute(
167-
"aria-sort", "ascending"
168-
)
166+
expect(page.locator("th", has_text="Pos").first).to_have_attribute("aria-sort", "ascending")
169167
page.screenshot(path=str(EVIDENCE / "position-asc-reset.png"), full_page=True)
170168
finally:
171169
context.close()
@@ -174,9 +172,7 @@ def test_sort_is_global_and_survives_pagination_without_javascript(
174172
def test_sort_is_keyboard_operable_and_announces_state(
175173
browser: Browser, live_server, sort_course: Cohort
176174
) -> None:
177-
page = studio_page(
178-
browser, live_server, sort_course, viewport={"width": 1440, "height": 900}
179-
)
175+
page = studio_page(browser, live_server, sort_course, viewport={"width": 1440, "height": 900})
180176
context = page.context
181177
try:
182178
settle(page, PAGE_SIZE)

playwright_tests/test_issue_237_qna_review.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,7 @@ def test_qna_participant_cohost_and_error_shells(
8484
assert detail.status == 200
8585
# UX-08 added the CSRF token as a hidden input: not an interactive
8686
# target, so the tap-target contract skips it.
87-
for target in page.locator(
88-
"button, textarea, input:not([type='hidden']), select"
89-
).all():
87+
for target in page.locator("button, textarea, input:not([type='hidden']), select").all():
9088
box = target.bounding_box()
9189
assert box is not None and box["height"] >= 44
9290
sort = page.locator("#qna-sort")

pyproject.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ module = [
158158
"scripts.projection_build.public_projection_source",
159159
"accounts.services.email_verification",
160160
"courses.services.mailchimp_course_tag_import",
161+
"courses.tests.test_enrollment_error_accessibility",
161162
"courses.tests.test_family_page_content",
162163
"courses.tests.test_homework_submission_learning_public_markup",
163164
"courses.tests.test_mailchimp_course_tag_import",
@@ -169,6 +170,7 @@ module = [
169170
"scripts.tests.test_ml_zoomcamp_2021_identity_merge",
170171
"scripts.tests.test_registrant_import",
171172
"scripts.tests.test_scoring_import_submitted_at",
173+
"studio_courses.tests.test_enrollment_sorting",
172174
]
173175
ignore_errors = false
174176

scripts/ci.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@
103103
"scripts/tests/test_identity_manifest.py",
104104
"scripts/tests/test_ml_zoomcamp_2021_identity_merge.py",
105105
"scripts/tests/test_registrant_import.py",
106+
"courses/tests/test_enrollment_error_accessibility.py",
107+
"studio_courses/tests/test_enrollment_sorting.py",
106108
)
107109
PRODUCTION_IMPORT_PYTHON: Final = (
108110
"scripts/prod",

studio_courses/tests/test_enrollment_sorting.py

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -107,43 +107,35 @@ def test_score_ties_keep_the_queryset_order(self):
107107
# Positions 2, 4 and 6 collapse onto one score (student-16 already
108108
# holds it): a stable sort must present the whole tied group in the
109109
# queryset's (position, id) order, after every higher score.
110-
Enrollment.objects.filter(
111-
course=self.course, position_on_leaderboard__in=[2, 4, 6]
112-
).update(total_score=15)
110+
Enrollment.objects.filter(course=self.course, position_on_leaderboard__in=[2, 4, 6]).update(
111+
total_score=15
112+
)
113113
ranked = usernames(self.ranked("total_score", "desc"))
114114
tied = [
115115
name
116116
for name in ranked
117117
if name in {"student-02", "student-04", "student-06", "student-16"}
118118
]
119-
self.assertEqual(
120-
tied, ["student-02", "student-04", "student-06", "student-16"]
121-
)
119+
self.assertEqual(tied, ["student-02", "student-04", "student-06", "student-16"])
122120

123121
def test_student_sort_is_case_insensitive(self):
124-
uppercase = Enrollment.objects.get(
125-
student__username="student-02", course=self.course
126-
)
122+
uppercase = Enrollment.objects.get(student__username="student-02", course=self.course)
127123
uppercase.student.username = "AAA-STUDENT"
128124
uppercase.student.save()
129125
ranked_enrollments = self.ranked("student", "asc")
130126
self.assertEqual(usernames(ranked_enrollments)[0], "AAA-STUDENT")
131127

132128
def test_no_sort_arguments_match_the_legacy_ordering(self):
133129
legacy = list(
134-
Enrollment.objects.filter(course=self.course).order_by(
135-
"position_on_leaderboard", "id"
136-
)
130+
Enrollment.objects.filter(course=self.course).order_by("position_on_leaderboard", "id")
137131
)
138132
self.assertEqual(
139133
usernames(self.ranked("position", "asc")),
140134
[enrollment.student.username for enrollment in legacy],
141135
)
142136

143137
def test_status_filter_and_sort_compose(self):
144-
hidden = Enrollment.objects.get(
145-
student__username="student-03", course=self.course
146-
)
138+
hidden = Enrollment.objects.get(student__username="student-03", course=self.course)
147139
hidden.display_on_leaderboard = False
148140
hidden.save()
149141
ranked_enrollments, counts = enrollment_list_data(
@@ -188,9 +180,7 @@ def test_descending_score_url_brings_the_global_top_to_page_one(self):
188180
self.assertEqual(self.first_row_username(response), "view-01")
189181

190182
def test_unknown_sort_param_is_refused_by_the_allowlist(self):
191-
response = self.client.get(
192-
self.url, {"sort": "student__password", "dir": "desc"}
193-
)
183+
response = self.client.get(self.url, {"sort": "student__password", "dir": "desc"})
194184
self.assertEqual(response.status_code, 200)
195185
# The column falls back to the default; an explicit direction is
196186
# still honored, so this is position descending.
@@ -239,9 +229,7 @@ def test_sorted_state_is_announced_in_the_results_line(self):
239229
self.assertContains(response, "sorted by total_score (desc)")
240230

241231
def test_second_page_of_a_sorted_view_is_the_sorted_sequence(self):
242-
response = self.client.get(
243-
self.url, {"sort": "total_score", "dir": "desc", "page": "2"}
244-
)
232+
response = self.client.get(self.url, {"sort": "total_score", "dir": "desc", "page": "2"})
245233
self.assertEqual(response.status_code, 200)
246234
page_two = usernames(response.context["enrollments"])
247235
self.assertEqual(len(page_two), 5)

0 commit comments

Comments
 (0)