Skip to content

Commit 7487e53

Browse files
MiodecMiodec
authored
refactor: result migration and tests cleanup (@Miodec) (monkeytypegame#6929)
move all legacy test value migration into a central place, update the tests to only check if that was called, test replaceLegacyValues
1 parent 4d46c62 commit 7487e53

File tree

6 files changed

+215
-182
lines changed

6 files changed

+215
-182
lines changed

‎backend/__tests__/__integration__/dal/result.spec.ts‎

Lines changed: 20 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as ResultDal from "../../../src/dal/result";
33
import { ObjectId } from "mongodb";
44
import * as UserDal from "../../../src/dal/user";
55
import { DBResult } from "../../../src/utils/result";
6+
import * as ResultUtils from "../../../src/utils/result";
67

78
let uid: string;
89
const timestamp = Date.now() - 60000;
@@ -64,11 +65,14 @@ async function createDummyData(
6465
}
6566
}
6667
describe("ResultDal", () => {
68+
const replaceLegacyValuesMock = vi.spyOn(ResultUtils, "replaceLegacyValues");
69+
6770
beforeEach(() => {
6871
uid = new ObjectId().toHexString();
6972
});
7073
afterEach(async () => {
7174
if (uid) await ResultDal.deleteAll(uid);
75+
replaceLegacyValuesMock.mockClear();
7276
});
7377
describe("getResults", () => {
7478
it("should read lastest 10 results ordered by timestamp", async () => {
@@ -135,84 +139,52 @@ describe("ResultDal", () => {
135139
expect(it.tags).toContain("old");
136140
});
137141
});
138-
it("should convert legacy values", async () => {
142+
it("should call replaceLegacyValues", async () => {
139143
//GIVEN
140-
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
144+
await createDummyData(uid, 1);
141145

142146
//WHEN
143-
const results = await ResultDal.getResults(uid);
147+
await ResultDal.getResults(uid);
144148

145149
//THEN
146-
expect(results[0]?.funbox).toEqual(["58008", "read_ahead"]);
150+
expect(replaceLegacyValuesMock).toHaveBeenCalled();
147151
});
148152
});
149153
describe("getResult", () => {
150-
it("should convert legacy values", async () => {
154+
it("should call replaceLegacyValues", async () => {
151155
//GIVEN
152-
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
156+
await createDummyData(uid, 1);
153157
const resultId = (await ResultDal.getLastResult(uid))._id.toHexString();
154158

155159
//WHEN
156-
const result = await ResultDal.getResult(uid, resultId);
160+
await ResultDal.getResult(uid, resultId);
157161

158162
//THEN
159-
expect(result?.funbox).toEqual(["58008", "read_ahead"]);
163+
expect(replaceLegacyValuesMock).toHaveBeenCalled();
160164
});
161165
});
162166
describe("getLastResult", () => {
163-
it("should convert legacy values", async () => {
167+
it("should call replaceLegacyValues", async () => {
164168
//GIVEN
165-
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
169+
await createDummyData(uid, 1);
166170

167171
//WHEN
168-
const result = await ResultDal.getLastResult(uid);
172+
await ResultDal.getLastResult(uid);
169173

170174
//THEN
171-
expect(result?.funbox).toEqual(["58008", "read_ahead"]);
175+
expect(replaceLegacyValuesMock).toHaveBeenCalled();
172176
});
173177
});
174178
describe("getResultByTimestamp", () => {
175-
it("should convert legacy values", async () => {
176-
//GIVEN
177-
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
178-
179-
//WHEN
180-
const result = await ResultDal.getResultByTimestamp(uid, timestamp);
181-
182-
//THEN
183-
expect(result?.funbox).toEqual(["58008", "read_ahead"]);
184-
});
185-
});
186-
describe("converts legacy values", () => {
187-
it("should convert funbox as string", async () => {
188-
//GIVEN
189-
await createDummyData(uid, 1, { funbox: "58008#read_ahead" as any });
190-
191-
//WHEN
192-
const read = await ResultDal.getLastResult(uid);
193-
194-
//THEN
195-
expect(read.funbox).toEqual(["58008", "read_ahead"]);
196-
});
197-
it("should convert funbox 'none'", async () => {
198-
//GIVEN
199-
await createDummyData(uid, 1, { funbox: "none" as any });
200-
201-
//WHEN
202-
const read = await ResultDal.getLastResult(uid);
203-
204-
//THEN
205-
expect(read.funbox).toEqual([]);
206-
});
207-
it("should not convert funbox as array", async () => {
179+
it("should call replaceLegacyValues", async () => {
208180
//GIVEN
209-
await createDummyData(uid, 1, { funbox: ["58008", "read_ahead"] });
181+
await createDummyData(uid, 1);
210182

211183
//WHEN
212-
const read = await ResultDal.getLastResult(uid);
184+
await ResultDal.getResultByTimestamp(uid, timestamp);
213185

214186
//THEN
215-
expect(read.funbox).toEqual(["58008", "read_ahead"]);
187+
expect(replaceLegacyValuesMock).toHaveBeenCalled();
216188
});
217189
});
218190
});

‎backend/__tests__/api/controllers/result.spec.ts‎

Lines changed: 0 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -271,44 +271,6 @@ describe("result controller test", () => {
271271
validationErrors: ["Unrecognized key(s) in object: 'extra'"],
272272
});
273273
});
274-
it("should get results with legacy values", async () => {
275-
//GIVEN
276-
const resultOne = givenDbResult(uid, {
277-
charStats: undefined,
278-
incorrectChars: 5,
279-
correctChars: 12,
280-
});
281-
const resultTwo = givenDbResult(uid, {
282-
charStats: undefined,
283-
incorrectChars: 7,
284-
correctChars: 15,
285-
});
286-
resultMock.mockResolvedValue([resultOne, resultTwo]);
287-
288-
//WHEN
289-
const { body } = await mockApp
290-
.get("/results")
291-
.set("Authorization", `Bearer ${uid}`)
292-
.send()
293-
.expect(200);
294-
295-
//THEN
296-
297-
expect(body.message).toEqual("Results retrieved");
298-
expect(body.data[0]).toMatchObject({
299-
_id: resultOne._id.toHexString(),
300-
charStats: [12, 5, 0, 0],
301-
});
302-
expect(body.data[0]).not.toHaveProperty("correctChars");
303-
expect(body.data[0]).not.toHaveProperty("incorrectChars");
304-
305-
expect(body.data[1]).toMatchObject({
306-
_id: resultTwo._id.toHexString(),
307-
charStats: [15, 7, 0, 0],
308-
});
309-
expect(body.data[1]).not.toHaveProperty("correctChars");
310-
expect(body.data[1]).not.toHaveProperty("incorrectChars");
311-
});
312274
it("should be rate limited", async () => {
313275
await expect(
314276
mockApp.get("/results").set("Authorization", `Bearer ${uid}`)
@@ -362,31 +324,6 @@ describe("result controller test", () => {
362324
.send()
363325
.expect(200);
364326
});
365-
it("should get last result with legacy values", async () => {
366-
//GIVEN
367-
const result = givenDbResult(uid, {
368-
charStats: undefined,
369-
incorrectChars: 5,
370-
correctChars: 12,
371-
});
372-
getResultMock.mockResolvedValue(result);
373-
374-
//WHEN
375-
const { body } = await mockApp
376-
.get(`/results/id/${result._id}`)
377-
.set("Authorization", `Bearer ${uid}`)
378-
.send()
379-
.expect(200);
380-
381-
//THEN
382-
expect(body.message).toEqual("Result retrieved");
383-
expect(body.data).toMatchObject({
384-
_id: result._id.toHexString(),
385-
charStats: [12, 5, 0, 0],
386-
});
387-
expect(body.data).not.toHaveProperty("correctChars");
388-
expect(body.data).not.toHaveProperty("incorrectChars");
389-
});
390327
it("should rate limit get result with ape key", async () => {
391328
//GIVEN
392329
const result = givenDbResult(uid, {
@@ -443,31 +380,6 @@ describe("result controller test", () => {
443380
.send()
444381
.expect(200);
445382
});
446-
it("should get last result with legacy values", async () => {
447-
//GIVEN
448-
const result = givenDbResult(uid, {
449-
charStats: undefined,
450-
incorrectChars: 5,
451-
correctChars: 12,
452-
});
453-
getLastResultMock.mockResolvedValue(result);
454-
455-
//WHEN
456-
const { body } = await mockApp
457-
.get("/results/last")
458-
.set("Authorization", `Bearer ${uid}`)
459-
.send()
460-
.expect(200);
461-
462-
//THEN
463-
expect(body.message).toEqual("Result retrieved");
464-
expect(body.data).toMatchObject({
465-
_id: result._id.toHexString(),
466-
charStats: [12, 5, 0, 0],
467-
});
468-
expect(body.data).not.toHaveProperty("correctChars");
469-
expect(body.data).not.toHaveProperty("incorrectChars");
470-
});
471383
it("should rate limit get last result with ape key", async () => {
472384
//GIVEN
473385
const result = givenDbResult(uid, {

‎backend/__tests__/utils/result.spec.ts‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import { describe, it, expect } from "vitest";
2+
import { replaceLegacyValues, DBResult } from "../../src/utils/result";
3+
4+
describe("Result Utils", () => {
5+
describe("replaceLegacyValues", () => {
6+
describe("legacy charStats conversion", () => {
7+
it.each([
8+
{
9+
description:
10+
"should convert correctChars and incorrectChars to charStats",
11+
correctChars: 95,
12+
incorrectChars: 5,
13+
expectedCharStats: [95, 5, 0, 0],
14+
},
15+
{
16+
description: "should handle zero values for legacy chars",
17+
correctChars: 0,
18+
incorrectChars: 0,
19+
expectedCharStats: [0, 0, 0, 0],
20+
},
21+
{
22+
description: "should handle large values for legacy chars",
23+
correctChars: 9999,
24+
incorrectChars: 1234,
25+
expectedCharStats: [9999, 1234, 0, 0],
26+
},
27+
])(
28+
"$description",
29+
({ correctChars, incorrectChars, expectedCharStats }) => {
30+
const resultWithLegacyChars: DBResult = {
31+
correctChars,
32+
incorrectChars,
33+
} as any;
34+
35+
const result = replaceLegacyValues(resultWithLegacyChars);
36+
37+
expect(result.charStats).toEqual(expectedCharStats);
38+
expect(result.correctChars).toBeUndefined();
39+
expect(result.incorrectChars).toBeUndefined();
40+
}
41+
);
42+
43+
it("should prioritise charStats when legacy data exists", () => {
44+
const resultWithBothFormats: DBResult = {
45+
charStats: [80, 4, 2, 1],
46+
correctChars: 95,
47+
incorrectChars: 5,
48+
} as any;
49+
50+
const result = replaceLegacyValues(resultWithBothFormats);
51+
52+
// Should convert legacy values and overwrite existing charStats
53+
expect(result.charStats).toEqual([80, 4, 2, 1]);
54+
// Legacy values should be removed after conversion
55+
expect(result.correctChars).toBeUndefined();
56+
expect(result.incorrectChars).toBeUndefined();
57+
});
58+
59+
it.each([
60+
{
61+
description:
62+
"should not convert when only one legacy property is present",
63+
input: { correctChars: 95 },
64+
expectedCharStats: undefined,
65+
expectedCorrectChars: 95,
66+
expectedIncorrectChars: undefined,
67+
},
68+
{
69+
description: "should not convert when only incorrectChars is present",
70+
input: { incorrectChars: 5 },
71+
expectedCharStats: undefined,
72+
expectedCorrectChars: undefined,
73+
expectedIncorrectChars: 5,
74+
},
75+
])(
76+
"$description",
77+
({
78+
input,
79+
expectedCharStats,
80+
expectedCorrectChars,
81+
expectedIncorrectChars,
82+
}) => {
83+
const result = replaceLegacyValues(input as any);
84+
85+
// Should not convert since both properties are required
86+
expect(result.charStats).toBe(expectedCharStats);
87+
expect(result.correctChars).toBe(expectedCorrectChars);
88+
expect(result.incorrectChars).toBe(expectedIncorrectChars);
89+
}
90+
);
91+
});
92+
93+
describe("legacy funbox conversion", () => {
94+
it.each([
95+
{
96+
description: "should convert string funbox to array",
97+
input: "memory#mirror",
98+
expected: ["memory", "mirror"],
99+
},
100+
{
101+
description: "should convert single funbox string to array",
102+
input: "memory",
103+
expected: ["memory"],
104+
},
105+
{
106+
description: "should convert 'none' funbox to empty array",
107+
input: "none",
108+
expected: [],
109+
},
110+
{
111+
description: "should handle complex funbox combinations",
112+
input: "memory#mirror#arrows#58008",
113+
expected: ["memory", "mirror", "arrows", "58008"],
114+
},
115+
])("$description", ({ input, expected }) => {
116+
const resultWithStringFunbox: DBResult = {
117+
funbox: input as any,
118+
} as any;
119+
120+
const result = replaceLegacyValues(resultWithStringFunbox);
121+
122+
expect(result.funbox).toEqual(expected);
123+
});
124+
});
125+
126+
it("should convert all legacy data at once", () => {
127+
const resultWithBothLegacy: DBResult = {
128+
correctChars: 100,
129+
incorrectChars: 8,
130+
funbox: "memory#mirror" as any,
131+
} as any;
132+
133+
const result = replaceLegacyValues(resultWithBothLegacy);
134+
135+
expect(result.charStats).toEqual([100, 8, 0, 0]);
136+
expect(result.correctChars).toBeUndefined();
137+
expect(result.incorrectChars).toBeUndefined();
138+
expect(result.funbox).toEqual(["memory", "mirror"]);
139+
});
140+
141+
describe("no legacy values", () => {
142+
it("should return result unchanged when no legacy values present", () => {
143+
const modernResult: DBResult = {
144+
charStats: [95, 5, 2, 1],
145+
funbox: ["memory"],
146+
} as any;
147+
148+
const result = replaceLegacyValues(modernResult);
149+
150+
expect(result).toEqual(modernResult);
151+
});
152+
});
153+
});
154+
});

0 commit comments

Comments
 (0)