Skip to content

Conversation

@hiento09
Copy link
Contributor

@hiento09 hiento09 commented Jul 22, 2025

This pull request introduces functionality to upload Jan application logs to ReportPortal, enhances log handling based on OS and version, and integrates log upload into the test result reporting workflow. The changes primarily focus on improving test reporting and log processing.

Enhancements to log handling and reporting:

  • Added log file path detection based on OS and version:
    The new get_jan_log_paths function dynamically determines Jan application log file paths for Windows, macOS, and Linux, accommodating both regular and nightly versions. Unsupported OSes are logged as warnings. (autoqa/reportportal_handler.py, autoqa/reportportal_handler.pyL163-R285)

  • Implemented log upload functionality:
    The upload_jan_logs function uploads Jan application logs to ReportPortal, including sorting logs by modification time, truncating oversized logs, and limiting the number of uploaded files. Logs are uploaded as text attachments with detailed logging for success and errors. (autoqa/reportportal_handler.py, autoqa/reportportal_handler.pyL163-R285)

Integration into test reporting workflow:

  • Integrated log upload into test result reporting:
    The upload_test_results_to_rp function now includes calls to upload_jan_logs, ensuring Jan application logs are uploaded alongside test results. (autoqa/reportportal_handler.py, autoqa/reportportal_handler.pyR406-R409)

  • Nightly version detection:
    Added logic to detect whether the Jan application is running in nightly mode based on the process name, enabling conditional log handling. (autoqa/test_runner.py, autoqa/test_runner.pyR28-R30)

  • Passed nightly mode flag to test result upload:
    Updated calls to upload_test_results_to_rp to include the is_nightly flag, ensuring correct log paths and handling. (autoqa/test_runner.py, autoqa/test_runner.pyL213-R216)


Important

Enhances Jan app log handling by uploading logs to ReportPortal and integrating them into test reporting, with dynamic log path detection and initialization wait in installation scripts.

  • Log Handling and Reporting:
    • get_jan_log_paths() in autoqa/reportportal_handler.py detects Jan log paths based on OS and version.
    • upload_jan_logs() uploads logs to ReportPortal, sorts by modification time, and limits file size and count.
  • Test Reporting Integration:
    • upload_test_results_to_rp() in autoqa/reportportal_handler.py integrates log uploads with test results.
    • Detects nightly mode in autoqa/test_runner.py and passes is_nightly flag to upload_test_results_to_rp().
  • Installation Scripts:
    • Adds 120-second wait for Jan app initialization in macos_install.sh, ubuntu_install.sh, and windows_install.ps1.

This description was created by Ellipsis for ca649eb. You can customize this summary. It will automatically update as commits are pushed.

@hiento09 hiento09 requested review from Minh141120 and louis-jan July 22, 2025 08:53
@hiento09 hiento09 self-assigned this Jul 22, 2025
@hiento09
Copy link
Contributor Author

image

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to acece9a in 1 minute and 59 seconds. Click for details.
  • Reviewed 176 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. autoqa/reportportal_handler.py:165
  • Draft comment:
    Consider using os.path.join to construct log file paths in get_jan_log_paths for better cross-platform handling.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. autoqa/reportportal_handler.py:239
  • Draft comment:
    Note that log truncation is based on the character count (len) rather than the actual byte size; non-ASCII characters may lead to discrepancies.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, explaining how log truncation works. It doesn't suggest any changes or ask for confirmation on specific code behavior.
3. autoqa/test_runner.py:29
  • Draft comment:
    The is_nightly flag is determined by a simple substring check on the process name; ensure this logic remains valid with all naming conventions.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_Uq1dqst1R1jbqRiA

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@hiento09 hiento09 force-pushed the chore/enrich-autoqa-log branch from acece9a to 1039f5b Compare July 22, 2025 08:59
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 1039f5b in 1 minute and 23 seconds. Click for details.
  • Reviewed 237 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. autoqa/reportportal_handler.py:170
  • Draft comment:
    Consider using os.path.join for constructing log file paths to ensure cross-platform compatibility instead of hardcoding separators.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. autoqa/reportportal_handler.py:240
  • Draft comment:
    Consider comparing file sizes using the byte size (e.g. via file_size) rather than len(log_content), as len returns character count which can differ with multi-byte characters.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. autoqa/utils.py:283
  • Draft comment:
    The additional wait time was reduced from 120s to 10s; ensure this shorter wait is sufficient in all environments to avoid race conditions.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_UWGWUPWrgy1kZlH6

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed ca649eb in 2 minutes and 8 seconds. Click for details.
  • Reviewed 36 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. autoqa/reportportal_handler.py:232
  • Draft comment:
    The PR description mentions truncating oversized logs, but this diff now skips any log file over 50MB instead of truncating it. Please confirm that this behavior is intentional and update the description or code comments accordingly.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This comment violates several rules: 1) It asks the author to confirm their intention, 2) It asks for PR description updates, 3) It's not clearly indicating a required code change. The behavior change itself (skipping vs truncating) could be valid - we don't have enough context to know if skipping large files is actually better than truncating them. The comment does identify a real change in behavior that could impact users. Maybe this is a legitimate regression that needs discussion? While the behavior change is real, the comment doesn't definitively show it's incorrect - it just asks for confirmation. If this was clearly a bug, the comment should directly state that and request a specific fix. Delete this comment. It violates multiple rules by asking for confirmation and PR description updates rather than clearly identifying a required code change.
2. autoqa/reportportal_handler.py:246
  • Draft comment:
    Since the code now fully reads log files (up to 50MB) into memory, consider if this could impact performance in low-memory environments. Using streaming or chunked reading might be beneficial if very large logs are expected.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None

Workflow ID: wflow_D05Q488K5A0MX8yc

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@hiento09 hiento09 merged commit 1dd5b81 into release/v0.6.6 Jul 22, 2025
5 of 8 checks passed
@hiento09 hiento09 deleted the chore/enrich-autoqa-log branch July 22, 2025 09:13
@github-project-automation github-project-automation bot moved this to QA in Jan Jul 22, 2025
@github-actions github-actions bot added this to the v0.6.6 milestone Jul 22, 2025
@hiento09 hiento09 moved this from QA to Done in Jan Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants