Skip to content

chore: Reduce lint exceptions in ctl #37643

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

Merged
sharat87 merged 8 commits into from
Nov 22, 2024
Merged

chore: Reduce lint exceptions in ctl #37643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
da8cfa6
Reduce lint exceptions in RTS
sharat87 Nov 22, 2024
2db58ac
Add Node types and remove ignore comments
sharat87 Nov 22, 2024
05d3b0c
Fix one more
sharat87 Nov 22, 2024
1bce028
Use same version of @types/node
sharat87 Nov 22, 2024
026adb7
chore: merge release branch
sharat87 Nov 22, 2024
73d044f
Discard changes to app/client/package.json
sharat87 Nov 22, 2024
7199aa1
Discard changes to app/client/yarn.lock
sharat87 Nov 22, 2024
1f2242a
deps
sharat87 Nov 22, 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
1 change: 1 addition & 0 deletions app/client/packages/rts/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"devDependencies": {
"@types/express": "^4.17.14",
"@types/jest": "^29.2.3",
"@types/node": "*",
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 22, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace wildcard version with a specific version for @types/node

Using "*" for @types/node could lead to unexpected type definition changes and potential conflicts. Consider using a specific version that matches your Node.js runtime version.

-    "@types/node": "*",
+    "@types/node": "^20.0.0"

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

💡 Codebase verification

Update @types/node to match Node.js v20.11.1

The project uses Node.js v20.11.1, but @types/node is set to "*". Using a wildcard version can lead to type definition mismatches. Update to a compatible version:

"@types/node": "^20.11.1"
🔗 Analysis chain

Verify Node.js version compatibility

Ensure the @types/node version matches your Node.js runtime version to prevent type definition mismatches.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check Node.js version in package.json and related files

# Check for Node.js version specifications in package.json or .nvmrc
fd -t f "package.json|.nvmrc" -x cat {} \; | grep -i "node"

# Check for Node.js engine constraints
cat package.json | jq '.engines.node'

Length of output: 1305

Script:

#!/bin/bash
# Let's check the root package.json and any .nvmrc files for Node.js version
cat app/client/package.json | jq '.engines.node'

# Also check the specific package's dependencies
cat app/client/packages/rts/package.json | jq '.dependencies["@types/node"], .devDependencies["@types/node"]'

Length of output: 180

"@types/nodemailer": "^6.4.17",
"@types/readline-sync": "^1.4.8",
"jest": "^29.3.1",
Expand Down
14 changes: 2 additions & 12 deletions app/client/packages/rts/src/ctl/.eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,6 @@
{
"extends": ["../../../../.eslintrc.base.json"],
"extends": ["../../.eslintrc.json"],
"rules": {
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/prefer-nullish-coalescing": "off",
"@typescript-eslint/strict-boolean-expressions": "off",
"@typescript-eslint/no-explicit-any": "off",
"testing-library/no-debugging-utils": "off",
"@typescript-eslint/no-var-requires": "off",
"padding-line-between-statements": "off",
"no-console": "off",
"@typescript-eslint/promise-function-async": "off",
"@typescript-eslint/no-unused-vars": "off",
"sort-destructure-keys/sort-destructure-keys": "off"
"no-console": "off"
}
}
29 changes: 28 additions & 1 deletion app/client/packages/rts/src/ctl/backup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ jest.mock("./utils", () => ({
import * as backup from "./backup";
import * as Constants from "./constants";
import os from "os";
// @ts-ignore
import fsPromises from "fs/promises";
import * as utils from "./utils";
import readlineSync from "readline-sync";
Expand All @@ -21,16 +20,19 @@ describe("Backup Tests", () => {

test("Available Space in /appsmith-stacks volume in Bytes", async () => {
const res = expect(await backup.getAvailableBackupSpaceInBytes("/"));

res.toBeGreaterThan(1024 * 1024);
});

it("Check the constant is 2 GB", () => {
const size = 2 * 1024 * 1024 * 1024;

expect(Constants.MIN_REQUIRED_DISK_SPACE_IN_BYTES).toBe(size);
});

it("Should throw Error when the available size is below MIN_REQUIRED_DISK_SPACE_IN_BYTES", () => {
const size = Constants.MIN_REQUIRED_DISK_SPACE_IN_BYTES - 1;

expect(() => backup.checkAvailableBackupSpace(size)).toThrow();
});

Expand All @@ -48,12 +50,14 @@ describe("Backup Tests", () => {
os.tmpdir = jest.fn().mockReturnValue("temp/dir");
fsPromises.mkdtemp = jest.fn().mockImplementation((a) => a);
const res = await backup.generateBackupRootPath();

expect(res).toBe("temp/dir/appsmithctl-backup-");
});

test("Test backup contents path generation", () => {
const root = "/rootDir";
const timestamp = "0000-00-0T00-00-00.00Z";

Copy link
Contributor

@coderabbitai coderabbitai bot Nov 22, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for special characters in paths

The command generation tests should include scenarios with paths containing spaces and special characters to ensure proper escaping.

Also applies to: 72-72, 94-94

expect(backup.getBackupContentsPath(root, timestamp)).toBe(
"/rootDir/appsmith-backup-0000-00-0T00-00-00.00Z",
);
Expand All @@ -65,6 +69,7 @@ describe("Backup Tests", () => {
const cmd =
"mongodump --uri=mongodb://username:password@host/appsmith --archive=/dest/mongodb-data.gz --gzip";
const res = await backup.executeMongoDumpCMD(dest, appsmithMongoURI);

expect(res).toBe(cmd);
console.log(res);
});
Expand All @@ -86,6 +91,7 @@ describe("Backup Tests", () => {
const dest = "/destdir";
const cmd = "ln -s /appsmith-stacks/git-storage /destdir/git-storage";
const res = await backup.executeCopyCMD(gitRoot, dest);

expect(res).toBe(cmd);
console.log(res);
});
Expand All @@ -99,6 +105,7 @@ describe("Backup Tests", () => {
}
});
const res = await utils.getCurrentAppsmithVersion();

expect(res).toBe("v0.0.0-SNAPSHOT");
});

Expand Down Expand Up @@ -130,70 +137,83 @@ describe("Backup Tests", () => {

test("Cleanup Backups when limit is 4 and there are 5 files", async () => {
const backupArchivesLimit = 4;

fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
const backupFiles = ["file1", "file2", "file3", "file4", "file5"];
const expectedBackupFiles = ["file2", "file3", "file4", "file5"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);

console.log(res);

expect(res).toEqual(expectedBackupFiles);
});

test("Cleanup Backups when limit is 2 and there are 5 files", async () => {
const backupArchivesLimit = 2;

fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
const backupFiles = ["file1", "file2", "file3", "file4", "file5"];
const expectedBackupFiles = ["file4", "file5"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);

console.log(res);

expect(res).toEqual(expectedBackupFiles);
});

test("Cleanup Backups when limit is 4 and there are 4 files", async () => {
const backupArchivesLimit = 4;

fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
const backupFiles = ["file1", "file2", "file3", "file4"];
const expectedBackupFiles = ["file1", "file2", "file3", "file4"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);

console.log(res);

expect(res).toEqual(expectedBackupFiles);
});

test("Cleanup Backups when limit is 4 and there are 2 files", async () => {
const backupArchivesLimit = 4;

fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
const backupFiles = ["file1", "file2"];
const expectedBackupFiles = ["file1", "file2"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);

console.log(res);

expect(res).toEqual(expectedBackupFiles);
});

test("Cleanup Backups when limit is 2 and there is 1 file", async () => {
const backupArchivesLimit = 4;

fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
const backupFiles = ["file1"];
const expectedBackupFiles = ["file1"];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);

console.log(res);
expect(res).toEqual(expectedBackupFiles);
});

test("Cleanup Backups when limit is 2 and there is no file", async () => {
const backupArchivesLimit = 4;

fsPromises.rm = jest.fn().mockImplementation(async (a) => console.log(a));
const backupFiles = [];
const expectedBackupFiles = [];
const res = await backup.removeOldBackups(backupFiles, backupArchivesLimit);

console.log(res);
expect(res).toEqual(expectedBackupFiles);
});

test("Test get encryption password from user prompt when both passwords are the same", async () => {
const password = "password#4321";

readlineSync.question = jest.fn().mockImplementation(() => password);
const password_res = backup.getEncryptionPasswordFromUser();

Expand All @@ -202,10 +222,12 @@ describe("Backup Tests", () => {

test("Test get encryption password from user prompt when both passwords are the different", async () => {
const password = "password#4321";

readlineSync.question = jest.fn().mockImplementation((a) => {
if (a == "Enter the above password again: ") {
return "pass";
}

return password;
});
const password_res = backup.getEncryptionPasswordFromUser();
Expand Down Expand Up @@ -233,6 +255,7 @@ describe("Backup Tests", () => {
archivePath,
encryptionPassword,
);

console.log(res);
expect(res).toEqual("/rootDir/appsmith-backup-0000-00-0T00-00-00.00Z.enc");
});
Expand All @@ -243,6 +266,7 @@ test("Get DB name from Mongo URI 1", async () => {
"mongodb+srv://admin:password@test.cluster.mongodb.net/my_db_name?retryWrites=true&minPoolSize=1&maxPoolSize=10&maxIdleTimeMS=900000&authSource=admin";
const expectedDBName = "my_db_name";
const dbName = utils.getDatabaseNameFromMongoURI(mongodb_uri);

expect(dbName).toEqual(expectedDBName);
});

Expand All @@ -251,6 +275,7 @@ test("Get DB name from Mongo URI 2", async () => {
"mongodb+srv://admin:password@test.cluster.mongodb.net/test123?retryWrites=true&minPoolSize=1&maxPoolSize=10&maxIdleTimeMS=900000&authSource=admin";
const expectedDBName = "test123";
const dbName = utils.getDatabaseNameFromMongoURI(mongodb_uri);

expect(dbName).toEqual(expectedDBName);
});

Expand All @@ -259,12 +284,14 @@ test("Get DB name from Mongo URI 3", async () => {
"mongodb+srv://admin:password@test.cluster.mongodb.net/test123";
const expectedDBName = "test123";
const dbName = utils.getDatabaseNameFromMongoURI(mongodb_uri);

expect(dbName).toEqual(expectedDBName);
});

test("Get DB name from Mongo URI 4", async () => {
const mongodb_uri = "mongodb://appsmith:pAssW0rd!@localhost:27017/appsmith";
const expectedDBName = "appsmith";
const dbName = utils.getDatabaseNameFromMongoURI(mongodb_uri);

expect(dbName).toEqual(expectedDBName);
});
Loading