Skip to content
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.

Sign up for GitHub

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

Feat/be/maintenance window module tests #1127

Merged
merged 2 commits into from
Nov 11, 2024
Merged

Feat/be/maintenance window module tests #1127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3496466
add missing await to
ajhollid Nov 8, 2024
247c034
Add tests for maintenance module
ajhollid Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Server/db/mongo/modules/maintenanceWindowModule.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ const deleteMaintenanceWindowByUserId = async (userId) => {

const editMaintenanceWindowById = async (maintenanceWindowId, maintenanceWindowData) => {
try {
const editedMaintenanceWindow = MaintenanceWindow.findByIdAndUpdate(
const editedMaintenanceWindow = await MaintenanceWindow.findByIdAndUpdate(
maintenanceWindowId,
maintenanceWindowData,
{ new: true }
Expand Down
267 changes: 267 additions & 0 deletions Server/tests/db/maintenanceWindowModule.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,267 @@
import sinon from "sinon";
import MaintenanceWindow from "../../db/models/MaintenanceWindow.js";
import {
createMaintenanceWindow,
getMaintenanceWindowById,
getMaintenanceWindowsByTeamId,
getMaintenanceWindowsByMonitorId,
deleteMaintenanceWindowById,
deleteMaintenanceWindowByMonitorId,
deleteMaintenanceWindowByUserId,
editMaintenanceWindowById,
} from "../../db/mongo/modules/maintenanceWindowModule.js";

describe("MaintenanceWindow Module", () => {
const mockMaintenanceWindow = {
monitorId: "123",
active: true,
oneTime: true,
start: 1,
end: 20000,
};

let mockMaintenanceWindows = [mockMaintenanceWindow];
let maintenanceWindowSaveStub,
maintenanceWindowFindByIdStub,
maintenanceWindowCountDocumentsStub,
maintenanceWindowFindStub,
maintenanceWindowFindByIdAndDeleteStub,
maintenanceWindowDeleteManyStub,
maintenanceWindowFindByIdAndUpdateStub;

beforeEach(() => {
maintenanceWindowSaveStub = sinon.stub(MaintenanceWindow.prototype, "save");
maintenanceWindowFindByIdStub = sinon.stub(MaintenanceWindow, "findById");
maintenanceWindowCountDocumentsStub = sinon.stub(MaintenanceWindow, "countDocuments");
maintenanceWindowFindStub = sinon.stub(MaintenanceWindow, "find").returns({
skip: sinon.stub().returns({
limit: sinon.stub().returns({
sort: sinon.stub().returns(mockMaintenanceWindows),
}),
}),
});
maintenanceWindowFindByIdAndDeleteStub = sinon.stub(
MaintenanceWindow,
"findByIdAndDelete"
);
maintenanceWindowDeleteManyStub = sinon.stub(MaintenanceWindow, "deleteMany");
maintenanceWindowFindByIdAndUpdateStub = sinon.stub(
MaintenanceWindow,
"findByIdAndUpdate"
);
});

afterEach(() => {
sinon.restore();
});

describe("createMaintenanceWindow", () => {
it("should save a new maintenance window", async () => {
maintenanceWindowSaveStub.resolves(mockMaintenanceWindow);
const result = await createMaintenanceWindow(mockMaintenanceWindow);
expect(result).to.deep.equal(mockMaintenanceWindow);
});

it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowSaveStub.rejects(err);
try {
await createMaintenanceWindow(mockMaintenanceWindow);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
});
Comment on lines +58 to +74
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Mom's spaghetti says we need more assertions!

The test suite should verify that the stub was called with the correct arguments:

 it("should save a new maintenance window", async () => {
     maintenanceWindowSaveStub.resolves(mockMaintenanceWindow);
     const result = await createMaintenanceWindow(mockMaintenanceWindow);
     expect(result).to.deep.equal(mockMaintenanceWindow);
+    expect(maintenanceWindowSaveStub.calledOnce).to.be.true;
+    expect(maintenanceWindowSaveStub.calledWith()).to.be.true;
 });

Committable suggestion skipped: line range outside the PR's diff.

describe("getMaintenanceWindowById", () => {
it("should return a maintenance window", async () => {
maintenanceWindowFindByIdStub.resolves(mockMaintenanceWindow);
const result = await getMaintenanceWindowById(mockMaintenanceWindow.id);
expect(result).to.deep.equal(mockMaintenanceWindow);
});
it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowFindByIdStub.rejects(err);
try {
await getMaintenanceWindowById(mockMaintenanceWindow.id);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
});

describe("getMaintenanceWindowsByTeamId", () => {
let query;
beforeEach(() => {
query = {
active: true,
page: 1,
rowsPerPage: 10,
field: "name",
order: "asc",
};
});

afterEach(() => {
sinon.restore();
});

it("should return a list of maintenance windows and count", async () => {
maintenanceWindowCountDocumentsStub.resolves(1);
const result = await getMaintenanceWindowsByTeamId(
mockMaintenanceWindow.teamId,
query
);
expect(result).to.deep.equal({
maintenanceWindows: mockMaintenanceWindows,
maintenanceWindowCount: 1,
});
});
Comment on lines +108 to +118
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Knees weak, arms heavy - we need to verify those query params!

The test should explicitly verify the query parameters passed to the stubs:

 it("should return a list of maintenance windows and count", async () => {
     maintenanceWindowCountDocumentsStub.resolves(1);
     const result = await getMaintenanceWindowsByTeamId(
         mockMaintenanceWindow.teamId,
         query
     );
+    expect(maintenanceWindowFindStub.calledWith({ teamId: mockMaintenanceWindow.teamId, active: true })).to.be.true;
+    expect(maintenanceWindowCountDocumentsStub.calledWith({ teamId: mockMaintenanceWindow.teamId, active: true })).to.be.true;
     expect(result).to.deep.equal({
         maintenanceWindows: mockMaintenanceWindows,
         maintenanceWindowCount: 1,
     });
 });

Committable suggestion skipped: line range outside the PR's diff.

it("should return a list of maintenance windows and count with empty query", async () => {
query = undefined;
maintenanceWindowCountDocumentsStub.resolves(1);
const result = await getMaintenanceWindowsByTeamId(
mockMaintenanceWindow.teamId,
query
);
expect(result).to.deep.equal({
maintenanceWindows: mockMaintenanceWindows,
maintenanceWindowCount: 1,
});
});
it("should return a list of maintenance windows and count with no pagination provided", async () => {
query.page = undefined;
query.rowsPerPage = undefined;
maintenanceWindowCountDocumentsStub.resolves(1);
const result = await getMaintenanceWindowsByTeamId(
mockMaintenanceWindow.teamId,
query
);
expect(result).to.deep.equal({
maintenanceWindows: mockMaintenanceWindows,
maintenanceWindowCount: 1,
});
});
it("should return a list of maintenance windows and count with field and desc order", async () => {
query.order = "desc";
maintenanceWindowCountDocumentsStub.resolves(1);
const result = await getMaintenanceWindowsByTeamId(
mockMaintenanceWindow.teamId,
query
);
expect(result).to.deep.equal({
maintenanceWindows: mockMaintenanceWindows,
maintenanceWindowCount: 1,
});
});
it("should return a list of maintenance windows and count no field", async () => {
query.field = undefined;
maintenanceWindowCountDocumentsStub.resolves(1);
const result = await getMaintenanceWindowsByTeamId(
mockMaintenanceWindow.teamId,
query
);
expect(result).to.deep.equal({
maintenanceWindows: mockMaintenanceWindows,
maintenanceWindowCount: 1,
});
});
it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowCountDocumentsStub.rejects(err);
try {
await getMaintenanceWindowsByTeamId(mockMaintenanceWindow.teamId, query);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
});
describe("getMaintenanceWindowsByMonitorId", () => {
it("should return a list of maintenance windows", async () => {
maintenanceWindowFindStub.resolves(mockMaintenanceWindows);
const result = await getMaintenanceWindowsByMonitorId(
mockMaintenanceWindow.monitorId
);
expect(result).to.deep.equal(mockMaintenanceWindows);
});
it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowFindStub.rejects(err);
try {
await getMaintenanceWindowsByMonitorId(mockMaintenanceWindow.monitorId);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
});
describe("deleteMaintenanceWindowById", () => {
it("should delete a maintenance window", async () => {
maintenanceWindowFindByIdAndDeleteStub.resolves(mockMaintenanceWindow);
const result = await deleteMaintenanceWindowById(mockMaintenanceWindow.id);
expect(result).to.deep.equal(mockMaintenanceWindow);
});
it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowFindByIdAndDeleteStub.rejects(err);
try {
await deleteMaintenanceWindowById(mockMaintenanceWindow.id);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
});

describe("deleteMaintenanceWindowByMonitorId", () => {
it("should return the number of documents deleted", async () => {
maintenanceWindowDeleteManyStub.resolves({ deletedCount: 1 });
const result = await deleteMaintenanceWindowByMonitorId(
mockMaintenanceWindow.monitorId
);
expect(result).to.deep.equal({ deletedCount: 1 });
});
it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowDeleteManyStub.rejects(err);
try {
await deleteMaintenanceWindowByMonitorId(mockMaintenanceWindow.monitorId);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
});

describe("deleteMaintenanceWindowByUserId", () => {
it("should return the number of documents deleted", async () => {
maintenanceWindowDeleteManyStub.resolves({ deletedCount: 1 });
const result = await deleteMaintenanceWindowByUserId(mockMaintenanceWindow.userId);
expect(result).to.deep.equal({ deletedCount: 1 });
});
it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowDeleteManyStub.rejects(err);
try {
await deleteMaintenanceWindowByUserId(mockMaintenanceWindow.userId);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
});
describe("editMaintenanceWindowById", () => {
it("should return the updated maintenance window", async () => {
maintenanceWindowFindByIdAndUpdateStub.resolves(mockMaintenanceWindow);
const result = await editMaintenanceWindowById(
mockMaintenanceWindow.id,
mockMaintenanceWindow
);
expect(result).to.deep.equal(mockMaintenanceWindow);
});
it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowFindByIdAndUpdateStub.rejects(err);
try {
await editMaintenanceWindowById(mockMaintenanceWindow.id, mockMaintenanceWindow);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
});
Comment on lines +248 to +266
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Palms are sweaty - we need to validate those update options!

The test should verify:

  • The update options (e.g., { new: true })
  • Partial updates
  • Non-existent ID handling
 it("should return the updated maintenance window", async () => {
     maintenanceWindowFindByIdAndUpdateStub.resolves(mockMaintenanceWindow);
     const result = await editMaintenanceWindowById(
         mockMaintenanceWindow.id,
         mockMaintenanceWindow
     );
+    expect(maintenanceWindowFindByIdAndUpdateStub.calledWith(
+        mockMaintenanceWindow.id,
+        mockMaintenanceWindow,
+        { new: true }
+    )).to.be.true;
     expect(result).to.deep.equal(mockMaintenanceWindow);
 });

+it("should handle non-existent maintenance window", async () => {
+    maintenanceWindowFindByIdAndUpdateStub.resolves(null);
+    const result = await editMaintenanceWindowById("non_existent_id", mockMaintenanceWindow);
+    expect(result).to.be.null;
+});

+it("should handle partial updates", async () => {
+    const partialUpdate = { active: false };
+    const updatedWindow = { ...mockMaintenanceWindow, ...partialUpdate };
+    maintenanceWindowFindByIdAndUpdateStub.resolves(updatedWindow);
+    const result = await editMaintenanceWindowById(mockMaintenanceWindow.id, partialUpdate);
+    expect(result).to.deep.equal(updatedWindow);
+});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe("editMaintenanceWindowById", () => {
it("should return the updated maintenance window", async () => {
maintenanceWindowFindByIdAndUpdateStub.resolves(mockMaintenanceWindow);
const result = await editMaintenanceWindowById(
mockMaintenanceWindow.id,
mockMaintenanceWindow
);
expect(result).to.deep.equal(mockMaintenanceWindow);
});
it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowFindByIdAndUpdateStub.rejects(err);
try {
await editMaintenanceWindowById(mockMaintenanceWindow.id, mockMaintenanceWindow);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
});
describe("editMaintenanceWindowById", () => {
it("should return the updated maintenance window", async () => {
maintenanceWindowFindByIdAndUpdateStub.resolves(mockMaintenanceWindow);
const result = await editMaintenanceWindowById(
mockMaintenanceWindow.id,
mockMaintenanceWindow
);
expect(maintenanceWindowFindByIdAndUpdateStub.calledWith(
mockMaintenanceWindow.id,
mockMaintenanceWindow,
{ new: true }
)).to.be.true;
expect(result).to.deep.equal(mockMaintenanceWindow);
});
it("should handle an error", async () => {
const err = new Error("test error");
maintenanceWindowFindByIdAndUpdateStub.rejects(err);
try {
await editMaintenanceWindowById(mockMaintenanceWindow.id, mockMaintenanceWindow);
} catch (error) {
expect(error).to.deep.equal(err);
}
});
it("should handle non-existent maintenance window", async () => {
maintenanceWindowFindByIdAndUpdateStub.resolves(null);
const result = await editMaintenanceWindowById("non_existent_id", mockMaintenanceWindow);
expect(result).to.be.null;
});
it("should handle partial updates", async () => {
const partialUpdate = { active: false };
const updatedWindow = { ...mockMaintenanceWindow, ...partialUpdate };
maintenanceWindowFindByIdAndUpdateStub.resolves(updatedWindow);
const result = await editMaintenanceWindowById(mockMaintenanceWindow.id, partialUpdate);
expect(result).to.deep.equal(updatedWindow);
});
});

});
Comment on lines +1 to +267
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Yo, these tests are fire, but they could use some coverage swagger! 🔥

The test suite is solid with good error handling and edge cases, but there's no test coverage configuration in sight. Consider adding:

  1. Create a jest.config.js in the root directory with coverage settings:
module.exports = {
  collectCoverage: true,
  coverageReporters: ['text', 'lcov'],
  collectCoverageFrom: [
    'Server/db/mongo/modules/*.js'
  ],
  coverageThreshold: {
    global: {
      branches: 80,
      functions: 80,
      lines: 80,
      statements: 80
    }
  }
}
  1. Add a coverage script to package.json:
{
  "scripts": {
    "test:coverage": "jest --coverage"
  }
}
🔗 Analysis chain

He's nervous, but on the surface the tests look calm and ready!

Consider adding test coverage configuration:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if Jest configuration exists
if [ -f "jest.config.js" ]; then
    echo "Jest config found. Add coverage configuration:"
    cat << 'EOF'
module.exports = {
  collectCoverage: true,
  coverageReporters: ['text', 'lcov'],
  collectCoverageFrom: [
    'Server/db/mongo/modules/maintenanceWindowModule.js'
  ],
  coverageThreshold: {
    global: {
      branches: 80,
      functions: 80,
      lines: 80,
      statements: 80
    }
  }
}
EOF
else
    echo "No Jest config found. Consider adding one for coverage reporting."
fi

Length of output: 168

Script:

#!/bin/bash
# Let's check if there are any test configuration files in the repository
fd -e config.js -e config.json -e config.ts

# Also check package.json for test configuration
if [ -f "package.json" ]; then
    echo "=== Package.json Jest Config ==="
    cat package.json | jq '.jest'
fi

# Check if there's any coverage reporting in existing test commands
if [ -f "package.json" ]; then
    echo "=== Test Scripts ==="
    cat package.json | jq '.scripts | select(. != null) | with_entries(select(.key | contains("test")))'
fi

Length of output: 119

Script:

#!/bin/bash
# Let's check the test files for any existing coverage configuration
rg -l "coverage|collectCoverage" Server/tests

# Check if there's a .nycrc or similar coverage config
fd -g '.nycrc*'
fd -g '.coverage*'

# Check if there are any test runner configurations in the Server directory
fd -g '*test*config*' Server/

Length of output: 534