-
Notifications
You must be signed in to change notification settings - Fork 1
Initial Commit on Lingoai > Login and Profile #133
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
base: lingo/main
Are you sure you want to change the base?
Conversation
/review |
Changelist by BitoThis pull request implements the following key changes.
|
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.
Code Review Agent Run #09aa03
Actionable Suggestions - 6
-
com.hybrid.joshsoftware.lingoai/src/main/java/Helper/Utility.java - 2
- Improper InterruptedException handling in pause method · Line 38-38
- Duplicate element lookups causing race conditions · Line 93-94
-
com.hybrid.joshsoftware.lingoai/src/test/java/testcases/ProfileIcon.java - 1
- Duplicate test priority values · Line 127-127
-
com.hybrid.joshsoftware.lingoai/src/main/java/Pages/HomePage.java - 1
- Incorrect null check causing NullPointerException · Line 143-143
-
com.hybrid.joshsoftware.lingoai/src/test/resources/config.properties - 2
- Missing output directory for report generation · Line 25-25
- Missing output directory for email reports · Line 35-35
Additional Suggestions - 8
-
com.hybrid.joshsoftware.lingoai/src/main/java/Helper/Utility.java - 1
-
Excessive 300 second timeout in checkElement · Line 49-49The `checkElement` method uses an extremely long timeout of 300 seconds (5 minutes) which is excessive for UI automation. This can cause tests to hang for very long periods if elements are not found. Consider reducing to a more reasonable timeout like 30-60 seconds.
Code suggestion
@@ -49,1 +49,1 @@ - WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(300)); + WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(30));
-
-
com.hybrid.joshsoftware.lingoai/src/main/java/Helper/DateUtility.java - 1
-
Package naming convention violation · Line 1-1Package name `Helper` violates Java naming conventions. Package names should be lowercase. Change to `helper` to follow standard conventions.
Code suggestion
@@ -1,1 +1,1 @@ -package Helper; +package helper;
-
-
com.hybrid.joshsoftware.lingoai/src/main/java/Pages/HomePage.java - 1
-
Overly generic XPath selector for file verification · Line 28-28XPath locator `//p` is too generic and may select unintended elements. Consider using a more specific selector to avoid false positives in file upload verification.
Code suggestion
@@ -28,1 +28,1 @@ - By verifyFileUploaded = By.xpath("//p"); + By verifyFileUploaded = By.xpath("//p[contains(@class,'upload-status') or contains(text(),'uploaded')]");
-
-
com.hybrid.joshsoftware.lingoai/src/test/java/testcases/LingoAILogin.java - 1
-
Fix test method description mismatch · Line 76-76Fix test method description mismatch: The method `validateMP3Upload()` has description 'Validate E2E after uploading MP4' but it actually validates MP3 upload. Change description to match the method name.
Code suggestion
@@ -76,1 +76,1 @@ - @Test(priority = 2, description = "Validate E2E after uploading MP4", dependsOnMethods = { "testMP3FileUpload" }) + @Test(priority = 2, description = "Validate E2E after uploading MP3", dependsOnMethods = { "testMP3FileUpload" })
-
-
com.hybrid.joshsoftware.lingoai/src/main/java/Listeners/MyTestNGListener.java - 1
-
Misleading log message in onTestStart · Line 16-16The log message in `onTestStart` method is misleading. It logs 'Test Pass' when a test is starting, not when it passes. This creates confusion in test reports. Change to 'Test STARTED' to accurately reflect the test lifecycle event.
Code suggestion
@@ -16,1 +16,1 @@ - ChainTestListener.log("Log:PASS - Test Pass " + result.getMethod().getMethodName()); + ChainTestListener.log("Log:START - Test STARTED " + result.getMethod().getMethodName());
-
-
com.hybrid.joshsoftware.lingoai/src/main/java/Helper/ConfigUtility.java - 1
-
Inefficient repeated file loading · Line 11-11Performance issue: Properties file is loaded on every method call instead of being cached. Consider loading once in static block or using singleton pattern to improve performance.
Code suggestion
@@ -9,16 +9,25 @@ public class ConfigUtility { + private static final Properties prop = loadProperties(); + + private static Properties loadProperties() { + Properties properties = new Properties(); + try (FileInputStream fis = new FileInputStream(new File(System.getProperty("user.dir") + "/Configuration/lingo.properties"))) { + properties.load(fis); + } catch (FileNotFoundException e) { + System.out.println("File Not Found " + e.getMessage()); + } catch (IOException e) { + System.out.println("Issue On loading " + e.getMessage()); + } + return properties; + } + public static String getProperty(String key) { - Properties prop = null; - - try { - - prop = new Properties(); - prop.load( - new FileInputStream(new File(System.getProperty("user.dir") + "/Configuration/lingo.properties"))); - } catch (FileNotFoundException e) { - System.out.println("File Not Found " + e.getMessage()); - } catch (IOException e) { - System.out.println("Issue On loading " + e.getMessage()); - } - return prop != null ? prop.getProperty(key) : null; + return prop.getProperty(key);
-
-
com.hybrid.joshsoftware.lingoai/src/test/java/testcases/ProfileIcon.java - 1
-
Inconsistent assertion library usage · Line 111-111Inconsistent assertion library usage: Using `AssertJUnit.assertEquals()` instead of `Assert.assertEquals()` like other assertions. This creates inconsistency and potential confusion. Use `Assert.assertEquals()` for consistency.
Code suggestion
@@ -111,1 +111,1 @@ -AssertJUnit.assertEquals(home.getLingoBotSubHeading(), "Do you want to add a bot for meeting Summarization?"); +Assert.assertEquals(home.getLingoBotSubHeading(), "Do you want to add a bot for meeting Summarization?");
-
-
com.hybrid.joshsoftware.lingoai/src/main/java/Pages/AudioResultPage.java - 1
-
Poor variable naming convention · Line 145-145Replace non-descriptive variable names `abc` and `xyz` with meaningful names like `conversationElements` and `element` to improve code readability and maintainability.
Code suggestion
@@ -145,4 +145,4 @@ - List<WebElement> abc = Utility.checkListElementPresence(driver, conversation_timeline_text_seq); - for (WebElement xyz : abc) { + List<WebElement> conversationElements = Utility.checkListElementPresence(driver, conversation_timeline_text_seq); + for (WebElement element : conversationElements) { WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(60)); - wait.until(ExpectedConditions.attributeContains(xyz, "class", "bg-green-100/50 border-green-50")); + wait.until(ExpectedConditions.attributeContains(element, "class", "bg-green-100/50 border-green-50"));
-
Review Details
-
Files reviewed - 18 · Commit Range:
300b57c..300b57c
- .gitignore
- com.hybrid.joshsoftware.lingoai/Configuration/lingo.properties
- com.hybrid.joshsoftware.lingoai/XMLFiles/e2e.xml
- com.hybrid.joshsoftware.lingoai/pom.xml
- com.hybrid.joshsoftware.lingoai/src/main/java/BaseClass/BaseClass.java
- com.hybrid.joshsoftware.lingoai/src/main/java/Helper/BrowserFactory.java
- com.hybrid.joshsoftware.lingoai/src/main/java/Helper/ConfigUtility.java
- com.hybrid.joshsoftware.lingoai/src/main/java/Helper/DateUtility.java
- com.hybrid.joshsoftware.lingoai/src/main/java/Helper/Utility.java
- com.hybrid.joshsoftware.lingoai/src/main/java/Listeners/MyTestNGListener.java
- com.hybrid.joshsoftware.lingoai/src/main/java/Pages/AudioResultPage.java
- com.hybrid.joshsoftware.lingoai/src/main/java/Pages/HomePage.java
- com.hybrid.joshsoftware.lingoai/src/main/java/Pages/LoginPage.java
- com.hybrid.joshsoftware.lingoai/src/main/java/dataproviders/DataProviders.java
- com.hybrid.joshsoftware.lingoai/src/main/java/dataproviders/ExcelUtility.java
- com.hybrid.joshsoftware.lingoai/src/test/java/testcases/LingoAILogin.java
- com.hybrid.joshsoftware.lingoai/src/test/java/testcases/ProfileIcon.java
- com.hybrid.joshsoftware.lingoai/src/test/resources/config.properties
-
Files skipped - 0
-
Tools
- Java-google-format (Linter) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Fb Infer (Static Code Analysis) - ✖︎ Failed
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review
- Manually triggers a full AI review. -
/pause
- Pauses automatic reviews on this pull request. -
/resume
- Resumes automatic reviews. -
/resolve
- Marks all Bito-posted review comments as resolved. -
/abort
- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent
You can customize the agent settings here or contact your Bito workspace admin at [email protected].
Documentation & Help
try { | ||
Thread.sleep(millisec); | ||
} catch (InterruptedException e) { | ||
System.out.println(e.getMessage()); |
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.
The pause
method catches InterruptedException
but only prints the message without restoring the interrupt status. This breaks proper thread interruption handling. Add Thread.currentThread().interrupt()
after logging to restore the interrupt status.
Code suggestion
Check the AI-generated fix before applying
System.out.println(e.getMessage()); | |
System.out.println(e.getMessage()); | |
Thread.currentThread().interrupt(); |
Code Review Run #09aa03
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
System.out.println(driver.findElement(locator).getText()); | ||
return Utility.checkElement(driver, locator).getText(); |
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.
The getElementText
method performs duplicate element lookups which can cause race conditions. It calls driver.findElement(locator).getText()
for logging, then Utility.checkElement(driver, locator).getText()
for the return value. This could lead to stale element references if the DOM changes between calls. Store the element text in a variable and use it for both logging and return.
Code suggestion
Check the AI-generated fix before applying
System.out.println(driver.findElement(locator).getText()); | |
return Utility.checkElement(driver, locator).getText(); | |
String elementText = Utility.checkElement(driver, locator).getText(); | |
System.out.println(elementText); | |
return elementText; |
Code Review Run #09aa03
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
*/ | ||
} | ||
|
||
@Test(priority = 3, dependsOnMethods = { "testProfileDropdownOptions" }) |
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.
Test priority conflict: Both testLingoBot()
and testLogout()
have the same priority value (3). This will cause unpredictable test execution order in TestNG. Change one test to priority 4 to ensure deterministic execution order.
Code suggestion
Check the AI-generated fix before applying
@Test(priority = 3, dependsOnMethods = { "testProfileDropdownOptions" }) | |
@Test(priority = 4, dependsOnMethods = { "testProfileDropdownOptions" }) |
Code Review Run #09aa03
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
String strB = arr[1]; | ||
actual_user_transcribe_details.add(strA); | ||
// Assert.assertNotNull(strB); | ||
if (strB.equals(null)) { |
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.
Incorrect null check using strB.equals(null)
will cause NullPointerException if strB
is null. Use strB == null
instead for proper null checking.
Code suggestion
Check the AI-generated fix before applying
if (strB.equals(null)) { | |
if (strB == null) { |
Code Review Run #09aa03
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
chaintest.generator.simple.enabled=true | ||
chaintest.generator.simple.document-title= LingoAI Automation Report | ||
chaintest.generator.simple.class-name=com.aventstack.chaintest.generator.ChainTestSimpleGenerator | ||
chaintest.generator.simple.output-file=Reports/chaintest/Index.html |
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.
The output file path Reports/chaintest/Index.html
references a directory that doesn't exist in the project structure. This will cause report generation to fail when the simple generator tries to write the HTML report.
Code suggestion
Check the AI-generated fix before applying
chaintest.generator.simple.output-file=Reports/chaintest/Index.html | |
chaintest.generator.simple.output-file=target/chaintest/Index.html |
Code Review Run #09aa03
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
chaintest.generator.email.enabled=true | ||
chaintest.generator.email.class-name=com.aventstack.chaintest.generator.ChainTestEmailGenerator | ||
chaintest.generator.email.output-file=Reports/chaintest/Email.html |
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.
The email generator output file path Reports/chaintest/Email.html
also references the same non-existent directory. This will cause email report generation to fail.
Code suggestion
Check the AI-generated fix before applying
chaintest.generator.email.output-file=Reports/chaintest/Email.html | |
chaintest.generator.email.output-file=target/chaintest/Email.html |
Code Review Run #09aa03
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Automation of
Summary by Bito
This pull request establishes the Lingoai automation framework with login and profile functionality testing capabilities. It introduces essential configurations in .gitignore, properties files, and pom.xml, while adding comprehensive page objects, utilities for data handling, and test cases for authentication and profile verification. The framework includes support for mp3 file uploads and reporting functionality.