Skip to content

Fix list accumulation in get_all_disk_paths#6284

Open
Poppy-mt wants to merge 1 commit intoavocado-framework:masterfrom
Poppy-mt:master
Open

Fix list accumulation in get_all_disk_paths#6284
Poppy-mt wants to merge 1 commit intoavocado-framework:masterfrom
Poppy-mt:master

Conversation

@Poppy-mt
Copy link

@Poppy-mt Poppy-mt commented Mar 10, 2026

Avoid reusing the same list across paths, which caused repeated
entries to be appended when iterating over multiple directories.

@gemini-code-assist
Copy link

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a subtle bug in the get_all_disk_paths utility function. Previously, the abs_path variable was inadvertently treated as a shared reference across loop iterations, leading to an accumulation of duplicate entries in the returned list of disk paths. The fix ensures that abs_path is correctly reinitialized for each directory processed, guaranteeing the accuracy and uniqueness of the collected disk paths without altering the function's public API.

Highlights

  • Bug Fix: Addressed an issue in get_all_disk_paths where the abs_path list was not reinitialized for each iteration, causing repeated entries to be appended to the final disk_list.
Changelog
  • avocado/utils/disk.py
    • Fixed an issue in get_all_disk_paths where the abs_path list was not properly reinitialized, leading to incorrect list accumulation.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@mr-avocado mr-avocado bot moved this to Review Requested in Default project Mar 10, 2026
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request modifies avocado/utils/disk.py to correct list initialization within the get_all_disk_paths function. It separates the initialization of disk_list and abs_path to prevent them from aliasing the same list object. Furthermore, abs_path is now re-initialized as an empty list inside the loop for each directory path, ensuring it correctly accumulates devices for the current path before extending the main disk_list, thereby fixing a potential bug related to incorrect path aggregation.

@Poppy-mt
Copy link
Author

@richtja Hi, I submitted this PR on March 10th, and it's been a week since then. I haven't received any feedback yet, so I'm reaching out to kindly ask if you might have some time to review it at your convenience.
Please feel free to let me know if there's any additional information I need to provide or any revisions required. I'm more than happy to make adjustments promptly.
Thank you so much for your time and assistance!

@richtja richtja requested a review from harvey0100 March 18, 2026 12:31
@Poppy-mt
Copy link
Author

@harvey0100 Kindly have it checked

@codecov
Copy link

codecov bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.70%. Comparing base (8fa8228) to head (5a055c0).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
avocado/utils/disk.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6284      +/-   ##
==========================================
- Coverage   73.70%   73.70%   -0.01%     
==========================================
  Files         206      206              
  Lines       22620    22621       +1     
==========================================
- Hits        16673    16672       -1     
- Misses       5947     5949       +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@harvey0100 harvey0100 left a comment

Choose a reason for hiding this comment

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

Hi @Poppy-mt, the fix is correct -- good catch on the list aliasing bug.

However, the commit is missing Signed-off-by and a commit body. Please amend it to follow the project's commit style (see styleguides).
For example:

avocado/utils/disk: fix list aliasing in get_all_disk_paths

The assignment disk_list = abs_path = [] creates a single list referenced by both names, causing entries to accumulate across loop iterations and duplicate on each extend call. Initialise disk_list separately and reset abs_path inside the loop.

Signed-off-by: Your Name your@email.com

Once that's done and CI is green, it LGTM.

The assignment disk_list = abs_path = [] creates a single list referenced by both names, causing entries to accumulate across loop iterations and duplicate on each extend call. Initialise disk_list separately and reset abs_path inside the loop.

Signed-off-by: Poppt-mt 935295021@qq.com
@Poppy-mt
Copy link
Author

Poppy-mt commented Mar 24, 2026

Hi @harvey0100, The failing CI test selftests/functional/job_timeout.py::JobTimeOutTest::test_sleep_short_timeout is unrelated to this change. The test expects exit code 8 (AVOCADO_JOB_INTERRUPTED), but in CI the job may exit with 9 (8 | 1) when a timeout interruption is also recorded as ERROR, so both AVOCADO_JOB_INTERRUPTED and AVOCADO_TESTS_FAIL are set. This appears to be an existing test/environment behavior, not a regression from this patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review Requested

Development

Successfully merging this pull request may close these issues.

2 participants