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

"upload-process --exclude" flags don't seem to work when uploading reports #482

Closed
mckern opened this issue Aug 7, 2024 · 3 comments · Fixed by #595
Closed

"upload-process --exclude" flags don't seem to work when uploading reports #482

mckern opened this issue Aug 7, 2024 · 3 comments · Fixed by #595
Assignees
Labels
bug Something isn't working high High Priority Issues (to be fixed within 2 sprints)

Comments

@mckern
Copy link

mckern commented Aug 7, 2024

While trying to exclude paths from search when running upload-process, I found that excluded paths were still being searched (and producing false-positive uploads of un-processable data). I tried passing them as relative and absolute paths before rolling up my sleeves and splashing a hundred logger.debug() calls through the code to trace it out. What I found follows:

When select_file_finder() is called, it gets a list of Path objects. But search_files() compares this as a list of strings; if d in folders_to_ignore is never true, so excluded paths still end up in the search tree when scanning for reports. I don't really know Python as much as I know how to be clumsy with Python but search_files() seems to expect a List(str). While I'm unfamiliar with Typing it looks like there's no implicit casting for a List(Path) to a List(str)? Here's a very, very trivial example I worked up based on the code from folder_searcher.py:

from pathlib import Path

paths = [Path("example1"), Path("example2"), Path("example3")]
strings = ["example1", "example2", "example3"]

matches = set(d for d in paths if d in strings)
print(f"paths that match: {matches}")
$ python3 example.py
paths that match: set()

Compared to:

from pathlib import Path

paths = [Path("example1"), Path("example2"), Path("example3")]
strings = ["example1", "example2", "example3"]

matches = set(d for d in paths if str(d) in strings)
print(f"paths that match: {matches}")
$ python3 example.py
paths that match: {PosixPath('example1'), PosixPath('example3'), PosixPath('example2')}

I'm not sure it's the cleanest way, but everything "just works" if I cast everything passed as folders_to_ignore to strings during FileFinder initialization but this feels kind of dodgy/hamfisted to me (again, not actually good at Python):

def select_file_finder(
    root_folder_to_search,
    folders_to_ignore,
    explicitly_listed_files,
    disable_search,
    report_type="coverage",
):
    # cast everything passed in folders_to_ignore as a string
    folders_to_ignore = [str(dir) for dir in folders_to_ignore]
    return FileFinder(
        root_folder_to_search,
        folders_to_ignore,
        explicitly_listed_files,
        disable_search,
        report_type,
    )
@mckern mckern changed the title "--exclude" flags don't seem to work when uploading reports "upload-process --exclude" flags don't seem to work when uploading reports Aug 7, 2024
@rohan-at-sentry rohan-at-sentry added the bug Something isn't working label Aug 23, 2024
@rohan-at-sentry rohan-at-sentry added the Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months label Sep 9, 2024
@thomasrockhu-codecov thomasrockhu-codecov self-assigned this Dec 2, 2024
@thomasrockhu-codecov thomasrockhu-codecov added high High Priority Issues (to be fixed within 2 sprints) and removed Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months labels Jan 13, 2025
@vlad-ko
Copy link

vlad-ko commented Jan 13, 2025

Just a quick note. After speaking with @thomasrockhu-codecov we now hope to have a patch deployed at the end of next week.

@thomasrockhu-codecov
Copy link
Contributor

Will be pushed in the next version of the CLI

@thomasrockhu-codecov
Copy link
Contributor

This should be fixed in the latest version, let me know if this is not the case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high High Priority Issues (to be fixed within 2 sprints)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants