Skip to content

test: integration tests for plans creation - testing course#250

Closed
simplyNoOne wants to merge 7 commits intoSolvro:mainfrom
simplyNoOne:test/create_plans
Closed

test: integration tests for plans creation - testing course#250
simplyNoOne wants to merge 7 commits intoSolvro:mainfrom
simplyNoOne:test/create_plans

Conversation

@simplyNoOne
Copy link
Copy Markdown
Member

@simplyNoOne simplyNoOne commented Jul 23, 2025

Important

This PR adds a comprehensive testing setup using Vitest, including new test scripts, dependencies, mock data, and tests for plan-related components and pages.

  • Testing Setup:
    • Adds vitest and related scripts code-test and code-test:ui to package.json.
    • Adds vitest.config.ts for Vitest configuration with plugins and setup files.
    • Introduces setup.ts for global test setup, including mock server initialization.
  • Dependencies:
    • Adds testing libraries: @testing-library/react, @testing-library/jest-dom, @testing-library/user-event, msw, jsdom, and vitest.
    • Adds mock service worker setup in server.ts and handlers in handlers.ts.
  • Mocks and Providers:
    • Adds Providers.tsx for wrapping components in necessary context providers during tests.
    • Adds mock data and functions in mock-data.ts, auth.ts, user.ts, and utils.tsx.
  • Component and Page Tests:
    • Adds editPlan.test.tsx for testing plan editing functionality.
    • Adds planPage.test.tsx for testing plan page interactions.
    • Updates app-sidebar.tsx and registration-combobox.tsx to include data-testid attributes for testing.

This description was created by Ellipsis for 5a4d5da. You can customize this summary. It will automatically update as commits are pushed.

Comment thread frontend/src/pages/editPlan.test.tsx Outdated
Comment thread frontend/src/pages/editPlan.test.tsx Outdated
Comment thread frontend/src/pages/planPage.test.tsx
Comment thread frontend/src/tests/mocks/handlers.ts Outdated
@simplyNoOne simplyNoOne reopened this Jul 25, 2025
Comment thread frontend/src/tests/mocks/mock-data.ts
@simplyNoOne simplyNoOne changed the title test: i burn with hate for this test: integration tests for plans creation - testing course Jul 26, 2025
Copy link
Copy Markdown
Member

@Rei-x Rei-x left a comment

Choose a reason for hiding this comment

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

mega duma Ania, że dałaś radę z przetestowaniem planera i to przy użyciu RTLa - wiele osób przy tym poległo 😭

z takich rzeczy co można by było poprawić to mocniejsze skupienie się na testowaniu behawioralnym, a nie testowaniu implementacji - nasze testy powinny wytrzymać 90% refactorów kodu jeśli dla użytkownika nic się nie zmieniło. Testowanie czy jakiś element html'a jest labelem czy spanem trochę temu przeczy. Ta zasada odnosi się wszędzie do testowania - także nawet we flutterze się przyda

tutaj fajny artykuł jak nie czujesz się jeszcze pewna co już jest implementation detail a co jeszcze nie:
https://blog.photogrammer.net/why-the-scope-of-your-tests-matter/ - trochę nacechowany C# ale dobrze opisuje o co chodzi

Comment on lines +23 to +25
await act(async () => {
await user.click(regOption);
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

potrzebny tu jest act?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

z tego co kojarze react testing library automatycznie wszystkie userEventy w to wrappuje

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Niewykluczone, że to pozostałość z moich gimnastyk, aby w ogóle te testy przeszły i akurat ta była niepotrzebna

const user = userEvent.setup();
render(
<Providers user={generateMockUser()} queryClient={new QueryClient()}>
<CreateNewPlanPage planId="testplan1" />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

obstawiam, że losowe generowanie id planu byłoby trochę bardziej bulletproof - na wypadek jakby coś od tego zależalo

const user = setup();

const label = screen.getByText("Wydział");
expect(label.tagName).toBe("LABEL");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

co tym właściwie testujesz?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

raczej to jest implementation detail

Comment on lines +115 to +136
export const mockFaculties: FacultyType = [
{
id: "id1",
name: "faculty 1",
departmentId: "dep1",
},
{
id: "id2",
name: "faculty 2",
departmentId: "dep2",
},
{
id: "id3",
name: "faculty 3",
departmentId: "dep2",
},
];

export const mockRegistrations: Registration[] = [
{ id: "reg1", name: "Registration 1" },
{ id: "reg2", name: "Registration 2" },
];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ja akurat jestem fanem nadawania jak najprawdziszych nazw dla testowych danych - czyli np. zamiast faculty1 to Wydział Informatyki i Telekomunikacji

łatwiej mi się wtedy testuje i myśli (z tego samego powodu mamy deskryptywne nazwy zmiennych, a nie x,y,z jak matematycy)

Comment on lines +89 to +94
const span1 = regs1.find(
(element) => element.tagName.toLowerCase() === "span",
);
const span2 = regs2.find(
(element) => element.tagName.toLowerCase() === "span",
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oj oj, co to właściwie testuje? czy istnieje span? to jest bardzo mocno testowanie implementation detail i nie powinno się tak robić - użytkownik nie patrzy na to czy dany element jest spanem

newPlan,
} from "./mock-data";

const NEXT_PUBLIC_API_URL = "http://localhost:3333";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hmm, to nie powinno być jakoś wyciągane ze zmiennych środowiskowych? pytam bo nie jestem pewien

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Wiem że coś mi nie chciało działać jak było w .env i chyba musiałam tak to zrobić, ale możliwe że coś przeoczyłam

Comment on lines +313 to +326
export const mockCourses: CourseType = [
{
id: "c1",
name: "Analizka",
registrationId: mockRegistrations[0].id,
groups: groupsForCourses1,
},
{
id: "c2",
name: "Algebra",
registrationId: mockRegistrations[0].id,
groups: groupsForCourses2,
},
];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
export const mockCourses: CourseType = [
{
id: "c1",
name: "Analizka",
registrationId: mockRegistrations[0].id,
groups: groupsForCourses1,
},
{
id: "c2",
name: "Algebra",
registrationId: mockRegistrations[0].id,
groups: groupsForCourses2,
},
];
export const mockCourses = [
{
id: "c1",
name: "Analizka",
registrationId: mockRegistrations[0].id,
groups: groupsForCourses1,
},
{
id: "c2",
name: "Algebra",
registrationId: mockRegistrations[0].id,
groups: groupsForCourses2,
},
] satisfies CourseType[];

satisfies jest troche bezpieczniejsze

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tyczy się też innych zmiennych

},
}));

vi.mock("next/headers", () => ({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nie mockowałaś już tego w frontend/src/tests/mocks/auth.ts?

ogólnie najlepiej trzymać wszystkie mocki w jednym miejscu według mnie, potem może się szybko pogubić co masz zamockowane a co nie

});
afterEach(() => {
server.resetHandlers();
cleanup();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

szacun, że pamiętałaś o cleanupie z rtl'a - większość zapominała

Comment thread frontend/package.json
Comment on lines +8 to +9
"code-test": "vitest",
"code-test:ui": "vitest --ui",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

raczej konwencja jest taka, że to się nazywa po prostu "test" i "test:ui". To że @qamarq wymyślił sobie, że skrypt "test" będzie robił linting to inna kwestia - to jakiś jego wymyśł.

w swoich projektach nazywaj to po prostu "test"

@qamarq qamarq closed this Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants