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.

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

ProgressBar_Enhancement #1613

Merged
merged 12 commits into from
Jun 5, 2024
Merged

Conversation

ahmednasr95
Copy link
Contributor

  • Fixed the repetitive logging during failed assertion
  • Added a new logger to the log4j2.properties to be able to print the progress bar on the console
  • Added the progress bar to indicate the time elapsed since start of assertion

* Added a new logger to the log4j2.properties to be able to print the progress bar on the console
* Added the progress bar to indicate the time elapsed since start of assertion
Copy link

codecov bot commented May 14, 2024

Codecov Report

Attention: Patch coverage is 82.92683% with 7 lines in your changes missing coverage. Please review.

Project coverage is 24.32%. Comparing base (0c51ab9) to head (994c307).

Current head 994c307 differs from pull request most recent head 12d72e7

Please upload reports for the commit 12d72e7 to get more accurate results.

Files Patch % Lines
...com/shaft/tools/io/internal/ProgressBarLogger.java 88.23% 3 Missing and 1 partial ⚠️
.../shaft/validation/internal/ValidationsHelper2.java 25.00% 3 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##               main    #1613       +/-   ##
=============================================
- Coverage     54.31%   24.32%   -30.00%     
+ Complexity     1377      633      -744     
=============================================
  Files           111      112        +1     
  Lines          9982    10012       +30     
  Branches        969      971        +2     
=============================================
- Hits           5422     2435     -2987     
- Misses         3947     7152     +3205     
+ Partials        613      425      -188     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MhmdElGazzar
Copy link
Contributor

@MohabMohie @MahmoudElSharkawy
could you please help us with this PR?

Copy link
Contributor

@MohabMohie MohabMohie left a comment

Choose a reason for hiding this comment

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

Thank you for an excellent implementation, and kindly find my comments inline.

regarding the virtual threads, kindly tag @MustafaAgamy via Slack and he'll be happy to help.

@@ -97,7 +97,7 @@ public static String convertBase64(String text) {
*/
public static int compareTwoObjects(Object expectedValue, Object actualValue, Object comparisonType,
Boolean validationType) {
ReportManager.logDiscrete("Expected \"" + expectedValue + "\", and actual \"" + actualValue + "\"");
//ReportManager.logDiscrete("Expected \"" + expectedValue + "\", and actual \"" + actualValue + "\"");
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of commenting this line, kindly log it using DEBUG level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to logging using the DEBUG level but to no avail. As shown in the screenshot below, The message is still logged to the console as an INFO.

Screenshot

src/test/java/poms/GoogleSearch.java Outdated Show resolved Hide resolved
src/test/java/poms/GoogleSearchResults.java Outdated Show resolved Hide resolved
src/test/resources/TestSuites/validationTests.xml Outdated Show resolved Hide resolved
ahmednasr95 and others added 7 commits May 24, 2024 15:43
…e condition.

* Returned the google search test cases to their initial state
* Added a virtual thread class that can be used to create virtual threads which assist in performing small tasks
…n the `ValidationsExecutor` class.

* Moved the stop thread method to after the switch statement mentioned above.
…m the log4j.properties file but to no avail

* This commit can be used to help in debugging the issue from multiple points of view.
…r logger is abstracted from the log4j2.properties file.

* The virtual thread setup and tear-down are returned to the ValidationsHelper2 because:
 1. The progress bar is needed only when a fluent wait is running.
 2. The progress bar logger configured just before the progress bar starts doesn't log into the external log file.
 3. Since the virtual thread tear-down is called after the switch statement, all information logged before that point is not logged into the log file which is undesirable behavior.
The aforementioned issue occurs only when the element being validated passes the validation after some time.
@MohabMohie MohabMohie marked this pull request as ready for review June 3, 2024 14:14
@MohabMohie MohabMohie self-requested a review June 3, 2024 14:14
@MohabMohie MohabMohie merged commit 18231e2 into ShaftHQ:main Jun 5, 2024
2 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants