Skip to content

Language tool files #33

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 52 additions & 0 deletions languageTool.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { runLanguageTool } from "./languageTool"
import fs from "fs"
import { env } from "../../utils/env"

const fileContent = fs.readFileSync("sample.txt", "utf-8")
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix the relative file path to be more robust.

The relative path "sample.txt" may fail if the test is executed from a different working directory. Consider using an absolute path or a path relative to the test file's location.

-const fileContent = fs.readFileSync("sample.txt", "utf-8")
+import path from "path"
+const fileContent = fs.readFileSync(path.join(__dirname, "sample.txt"), "utf-8")
🤖 Prompt for AI Agents
In languageTool.test.ts at line 5, the file path "sample.txt" is relative and
may break if the test runs from a different directory. Update the path to be
absolute or relative to the test file's location by using __dirname or
path.resolve to construct the file path, ensuring it correctly locates
"sample.txt" regardless of the working directory.


const mockParams = {
logger: {
child: () => ({
debug: console.log,
error: console.error,
}),
},
settings: {
configSettings: {
reviews: {
profile: "chill", // or "assertive"
tools: {
languagetool: {
enabled: true,
disabled_categories: [],
disabled_rules: [],
enabled_categories: [],
enabled_rules: [],
enabled_only: false,
level: "default",
},
},
},
language: "en-US",
},
},
fileHunks: [
{
repoFile: {
filename: "sample.txt",
},
fileContent,
hunks: [],
toolOutputs: {},
},
],
} as any

// Fake credentials for testing (you should replace these with valid test creds or mock the API)
env.LANGUAGETOOL_USERNAME = "fakeUser"
env.LANGUAGETOOL_API_KEY = "fakeKey"
Comment on lines +46 to +47
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid direct environment variable mutation in tests.

Directly mutating environment variables can cause issues in parallel test execution and doesn't clean up after the test. Consider using a mocking library or proper test setup/teardown.

-// Fake credentials for testing (you should replace these with valid test creds or mock the API)
-env.LANGUAGETOOL_USERNAME = "fakeUser"
-env.LANGUAGETOOL_API_KEY = "fakeKey"
+// Mock environment variables for testing
+const originalUsername = env.LANGUAGETOOL_USERNAME
+const originalApiKey = env.LANGUAGETOOL_API_KEY
+
+env.LANGUAGETOOL_USERNAME = "fakeUser"
+env.LANGUAGETOOL_API_KEY = "fakeKey"

And add cleanup after the test:

 runLanguageTool(mockParams).then(() => {
 	console.log("Test complete. Tool outputs:", mockParams.fileHunks[0].toolOutputs)
+	// Restore original environment variables
+	env.LANGUAGETOOL_USERNAME = originalUsername
+	env.LANGUAGETOOL_API_KEY = originalApiKey
 })
🤖 Prompt for AI Agents
In languageTool.test.ts around lines 46 to 47, avoid directly mutating
environment variables as it can cause issues in parallel tests and lacks
cleanup. Instead, use a mocking library or test setup/teardown methods to
temporarily set LANGUAGETOOL_USERNAME and LANGUAGETOOL_API_KEY for the test, and
ensure you restore or clean up these environment variables after the test
completes.


runLanguageTool(mockParams).then(() => {
console.log("Test complete. Tool outputs:", mockParams.fileHunks[0].toolOutputs)
})
Comment on lines +49 to +51
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add proper error handling and test structure.

The test lacks error handling and doesn't follow standard testing patterns. Consider using a proper test framework with describe/it blocks and assertions.

-runLanguageTool(mockParams).then(() => {
-	console.log("Test complete. Tool outputs:", mockParams.fileHunks[0].toolOutputs)
-})
+describe("runLanguageTool", () => {
+	it("should detect language errors in sample text", async () => {
+		try {
+			await runLanguageTool(mockParams)
+			const toolOutputs = mockParams.fileHunks[0].toolOutputs
+			
+			// Add assertions here to verify expected behavior
+			expect(toolOutputs).toBeDefined()
+			expect(toolOutputs.languagetool).toBeDefined()
+			
+			console.log("Test complete. Tool outputs:", toolOutputs)
+		} catch (error) {
+			console.error("Test failed:", error)
+			throw error
+		}
+	})
+})
📝 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
runLanguageTool(mockParams).then(() => {
console.log("Test complete. Tool outputs:", mockParams.fileHunks[0].toolOutputs)
})
describe("runLanguageTool", () => {
it("should detect language errors in sample text", async () => {
try {
await runLanguageTool(mockParams)
const toolOutputs = mockParams.fileHunks[0].toolOutputs
// Add assertions here to verify expected behavior
expect(toolOutputs).toBeDefined()
expect(toolOutputs.languagetool).toBeDefined()
console.log("Test complete. Tool outputs:", toolOutputs)
} catch (error) {
console.error("Test failed:", error)
throw error
}
})
})
🤖 Prompt for AI Agents
In languageTool.test.ts around lines 49 to 51, the test currently uses a simple
promise with console.log and lacks error handling and proper test structure.
Refactor the test to use a testing framework like Jest or Mocha with describe
and it blocks, include assertions to verify expected outcomes, and add error
handling to catch and report any failures during the runLanguageTool execution.


10 changes: 10 additions & 0 deletions sample.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
This is a test file with some intentional errors.

Their going to the park later, maybe I'll join to.
This sentence have a agreement error.
The list are long and detailed.
He don't know what he doing.
Here is a mispeling.

It is an very good idea to try this tool, isn't it.