326 muc business hours tests #337
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new configuration file named Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
src/components/BusinessHours/MucBusinessHours.test.ts (6)
7-7: Ze type annotation could be more specific, ja!Instead of using
Array<any>, ve should define a proper interface for ze business hours.- let businessHours : Array<any>; + interface BusinessHour { + weekDay: string; + openingHours: Array<{ from: string; to: string; }>; + } + let businessHours: BusinessHour[];
21-30: Ja, ze test looks gut, but ve should add accessibility check!Ze toggle button should have proper ARIA attributes for better accessibility.
expect(wrapper.find('.m-business-hours-toggle__trigger').exists()).toBe(true); + expect(wrapper.find('.m-business-hours-toggle__trigger').attributes('aria-expanded')).toBe('false');
49-49: Ze comments should be in English, nicht Deutsch!For international collaboration, ve should use English comments.
- expect(wrapper.vm.collapsed).toBe(true); // Initialzustand + expect(wrapper.vm.collapsed).toBe(true); // Initial state
105-117: Gut test, aber ve can improve ze accessibility testing!Ze hint should have proper ARIA attributes for screen readers.
- hint: () => h('div', 'This is a hint'), + hint: () => h('div', { + class: 'hint', + role: 'note', + 'aria-label': 'Additional information' + }, 'This is a hint'),
119-131: Ja, ze test is gut, aber ve need error cases!Ve should also test invalid prop values und error handling.
+ it('handles invalid businessHours prop gracefully', async () => { + const wrapper = mount(BusinessHoursToggle, { + props: { + businessHours: null, + toggleable: true, + }, + }); + expect(wrapper.find('.error-message').exists()).toBe(true); + });
1-132: Sehr gut test suite, but ve can add more edge cases!Consider adding these additional test cases:
- Multiple opening hours per day
- Invalid time formats
- Empty businessHours array
- Performance with large datasets
Would you like me to help generate these additional test cases?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.coderabbit.yaml(1 hunks)src/components/BusinessHours/MucBusinessHours.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .coderabbit.yaml
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
src/components/BusinessHours/MucBusinessHours.test.ts (3)
5-5: Ze comment about adjusting ze path should be removed!Ze comment "// Pfad zur Komponente anpassen" is unnecessary since ze path is already correct.
22-45: Ze tests are gut, but could be more specific!Ve should also verify zat ze toggle button has ze correct aria-label for accessibility.
expect(wrapper.find(".m-business-hours-toggle__trigger").exists()).toBe( true ); + expect(wrapper.find(".m-business-hours-toggle__trigger").attributes('aria-label')).toBeDefined();
127-139: Ve should test ze initial state ven toggleable is false!Add a test case for ze initial state ven toggleable prop is false:
+ it("starts uncollapsed when toggleable is false", () => { + const wrapper = mount(BusinessHoursToggle, { + props: { + businessHours, + toggleable: false, + }, + }); + expect((wrapper.vm as BusinessHoursToggleInstance).collapsed).toBe(false); + });🧰 Tools
🪛 GitHub Check: lint
[failure] 134-134:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 136-136:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/BusinessHours/MucBusinessHours.test.ts(1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/BusinessHours/MucBusinessHours.test.ts (1)
Learnt from: langehm
PR: it-at-m/muc-patternlab-vue#337
File: src/components/BusinessHours/MucBusinessHours.test.ts:74-76
Timestamp: 2024-11-27T14:51:18.401Z
Learning: In the `MucBusinessHours` component (`src/components/BusinessHours/MucBusinessHours.vue`), ze component uses ze current time, so tests do not need to mock ze date.
🪛 GitHub Check: lint
src/components/BusinessHours/MucBusinessHours.test.ts
[failure] 54-54:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 56-56:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 58-58:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 68-68:
Property 'collapseClass' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 69-69:
Property 'toggleCollapsable' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 70-70:
Property 'collapseClass' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 81-81:
Property 'todaysBusinessHours' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'. Did you mean 'businessHours'?
[failure] 94-94:
Parameter 'time' implicitly has an 'any' type.
[failure] 134-134:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 136-136:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/components/BusinessHours/MucBusinessHours.test.ts (1)
15-23: Ve should add more test cases!Ze test data could include more edge cases:
- Multiple opening hours per day (e.g., morning and afternoon shifts)
- Special hours (e.g., 24-hour service)
beforeEach(() => { businessHours = [ { weekDay: "Mo", openingHours: [{ from: "09:00", to: "17:00" }] }, { weekDay: "Di", openingHours: [{ from: "09:00", to: "17:00" }] }, { weekDay: "Mi", openingHours: [] }, { weekDay: "Do", openingHours: [{ from: "09:00", to: "17:00" }] }, - { weekDay: "Fr", openingHours: [{ from: "09:00", to: "17:00" }] }, + { weekDay: "Fr", openingHours: [ + { from: "09:00", to: "12:00" }, + { from: "13:00", to: "17:00" } + ] }, { weekDay: "Sa", openingHours: [] }, { weekDay: "So", openingHours: [] }, ]; });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/BusinessHours/MucBusinessHours.test.ts(1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/BusinessHours/MucBusinessHours.test.ts (1)
Learnt from: langehm
PR: it-at-m/muc-patternlab-vue#337
File: src/components/BusinessHours/MucBusinessHours.test.ts:74-76
Timestamp: 2024-11-27T14:51:18.401Z
Learning: In the `MucBusinessHours` component (`src/components/BusinessHours/MucBusinessHours.vue`), ze component uses ze current time, so tests do not need to mock ze date.
🪛 GitHub Check: lint
src/components/BusinessHours/MucBusinessHours.test.ts
[failure] 29-29:
Type 'BusinessHour[]' is not assignable to type 'BusinessHourType[]'.
[failure] 42-42:
Type 'BusinessHour[]' is not assignable to type 'BusinessHourType[]'.
[failure] 54-54:
Type 'BusinessHour[]' is not assignable to type 'BusinessHourType[]'.
[failure] 58-58:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 60-60:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 62-62:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 68-68:
Type 'BusinessHour[]' is not assignable to type 'BusinessHourType[]'.
[failure] 72-72:
Property 'collapseClass' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 73-73:
Property 'toggleCollapsable' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 74-74:
Property 'collapseClass' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
🔇 Additional comments (1)
src/components/BusinessHours/MucBusinessHours.test.ts (1)
51-63:
Ze component instance needs proper typing!
Ve must properly type ze component instance to access its properties.
Previous suggestion vas to add:
+import { ComponentPublicInstance } from 'vue'
+interface BusinessHoursToggleInstance extends ComponentPublicInstance {
+ collapsed: boolean;
+ collapseClass: string;
+ toggleCollapsable: () => void;
+}
-expect(wrapper.vm.collapsed).toBe(true);
+expect((wrapper.vm as BusinessHoursToggleInstance).collapsed).toBe(true);Also applies to: 133-145
🧰 Tools
🪛 GitHub Check: lint
[failure] 54-54:
Type 'BusinessHour[]' is not assignable to type 'BusinessHourType[]'.
[failure] 58-58:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 60-60:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
[failure] 62-62:
Property 'collapsed' does not exist on type 'ComponentPublicInstance<NonNullable<{ readonly businessHours: BusinessHourType[]; readonly toggleable?: boolean | undefined; readonly icon?: string | undefined; } & VNodeProps & AllowedComponentProps & ComponentCustomProps>, { ...; } & ... 7 more ... & Omit<...>>'.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
src/components/BusinessHours/MucBusinessHours.test.ts (1)
1-6: Ze imports could be better organized!Consider grouping ze imports by external and internal dependencies:
-import { mount } from "@vue/test-utils"; -import { beforeEach, describe, expect, it } from "vitest"; -import {ComponentPublicInstance, h} from "vue"; - -import BusinessHoursToggle from "./MucBusinessHours.vue"; -import {BusinessHourType} from "./BusinessHourType"; +// External dependencies +import { mount } from "@vue/test-utils"; +import { beforeEach, describe, expect, it } from "vitest"; +import { ComponentPublicInstance, h } from "vue"; + +// Internal dependencies +import BusinessHoursToggle from "./MucBusinessHours.vue"; +import { BusinessHourType } from "./BusinessHourType";
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/BusinessHours/MucBusinessHours.test.ts(1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/BusinessHours/MucBusinessHours.test.ts (1)
Learnt from: langehm
PR: it-at-m/muc-patternlab-vue#337
File: src/components/BusinessHours/MucBusinessHours.test.ts:74-76
Timestamp: 2024-11-27T14:51:18.401Z
Learning: In the `MucBusinessHours` component (`src/components/BusinessHours/MucBusinessHours.vue`), ze component uses ze current time, so tests do not need to mock ze date.
🔇 Additional comments (1)
src/components/BusinessHours/MucBusinessHours.test.ts (1)
18-28: 🛠️ Refactor suggestion
Ve need more test cases!
Ze test data should include more edge cases:
beforeEach(() => {
businessHours = [
{ weekDay: "Mo", openingHours: [{ from: "09:00", to: "17:00" }] },
{ weekDay: "Di", openingHours: [{ from: "09:00", to: "17:00" }] },
{ weekDay: "Mi", openingHours: [] },
{ weekDay: "Do", openingHours: [{ from: "09:00", to: "17:00" }] },
{ weekDay: "Fr", openingHours: [{ from: "09:00", to: "17:00" }] },
{ weekDay: "Sa", openingHours: [] },
{ weekDay: "So", openingHours: [] },
+ // Add edge cases
+ { weekDay: "Mo", openingHours: [
+ { from: "09:00", to: "12:00" },
+ { from: "13:00", to: "17:00" }
+ ]}, // Multiple time slots
+ { weekDay: "Di", openingHours: [{ from: "00:00", to: "24:00" }] }, // 24-hour
+ { weekDay: "Mi", openingHours: [{ from: "18:00", to: "03:00" }] }, // Overnight
];
});Likely invalid or redundant comment.
|
😂 |
Description
Add tests for MucBusinessHours.
Reference
Issues #326
Summary by CodeRabbit
New Features
Tests