-
Notifications
You must be signed in to change notification settings - Fork 13k
test: Fix ignored unit tests #38288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: Fix ignored unit tests #38288
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughThis PR updates Mocha test configuration and refactors multiple test files for Livechat contact functionality, including test fixture enhancements with lastChat fields, API signature adjustments for customFields handling, and test expectation updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38288 +/- ##
===========================================
+ Coverage 70.75% 70.82% +0.06%
===========================================
Files 3142 3158 +16
Lines 108926 109363 +437
Branches 19581 19682 +101
===========================================
+ Hits 77072 77451 +379
- Misses 29857 29876 +19
- Partials 1997 2036 +39
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/meteor/app/livechat/server/lib/contacts/validateCustomFields.spec.ts (1)
54-61: Test description doesn't match the option name used.The test description on line 54 says "ignoreValidationErrors option is provided" but the code on line 58 uses
ignoreAdditionalFields. This inconsistency could cause confusion when debugging test failures.Suggested fix
- it('should not throw an error if a extra custom field is passed, but the ignoreValidationErrors option is provided', () => { + it('should not throw an error if an extra custom field is passed, but the ignoreAdditionalFields option is provided', () => {apps/meteor/app/livechat/server/lib/contacts/resolveContactConflicts.spec.ts (3)
86-121: Nestedit()blocks will never execute.The inner
it()block on line 87 is nested inside the outerit()on line 86. Mocha does not support this pattern - nestedit()calls are silently ignored. This test will appear to pass but never actually run the assertions.Suggested fix: flatten the test structure
- it('should wipe conflicts if wipeConflicts = true', async () => { - it('should update the contact with the resolved name', async () => { + it('should wipe conflicts if wipeConflicts = true', async () => { modelsMock.LivechatContacts.findOneEnabledById.resolves({ // ... rest of test setup }); // ... assertions - }); });Remove the inner
it()wrapper and keep only the outer test block with its assertions.
123-158: Duplicate test description and nestedit()pattern.This test has the same issues as lines 86-121:
- The description duplicates "should wipe conflicts if wipeConflicts = true" but tests with
wipeConflicts: false(line 142)- Contains a nested
it()block that will never executeThe test description should reflect the actual scenario being tested (e.g., "should preserve remaining conflicts if wipeConflicts = false").
160-179: Fix parameter key: changecustomFieldtocustomFieldson lines 162 and 176.The function signature uses
customFields(plural). Lines 40 and 76 correctly passcustomFields, but lines 162 and 176 incorrectly usecustomField(singular). This should becustomFields: { customField: 'newValue' }to match the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 5 files
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.