fix: len(None) bug

[joseph-sentry]

when the versioning system is None, list_relevant_files returns None so this changes the code in the upload_collector and the network_finder to handle that case

Fixes: #641

[sentry-io]

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: codecov_cli/services/upload/upload_collector.py

Function	Unhandled Issue
generate_upload_data	TypeError: object of type 'NoneType' has no len() Upload Coverag... Event Count: 348
generate_upload_data	TypeError: can only concatenate list (not "tuple") to list codecov_cli.services.upload.file_finder in ... Event Count: 12
generate_upload_data	TypeError: 'NoneType' object is not iterable Uplo... Event Count: 2
generate_upload_data	TypeError: object of type 'NoneType' has no len() Do Uploa... Event Count: 2

---

_{Did you find this useful? React with a 👍 or 👎}

[codecov]

❌ 5 Tests Failed:

Tests completed	Failed	Passed	Skipped
3570	5	3565	0

View the top 3 failed test(s) by shortest run time

api.temp.calculator.test_calculator::test_divide

Stack Traces | 0.001s run time

def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError

api.temp.calculator.test_calculator::test_divide

Stack Traces | 0.001s run time

def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError

api.temp.calculator.test_calculator::test_divide

Stack Traces | 0.001s run time

def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

[github-actions]

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

[thomasrockhu-codecov]

@joseph-sentry I was just gonna do https://github.com/codecov/codecov-cli/pull/642/files#diff-d906bb99e8dde0f79f239d763518098ef291b888fb5318ecc5c75aea9b80916aL29

[Dreamsorcerer]

Is mypy run on this project?
If files is None (as described in the PR description), your changed code surely results in returning None, in which case this annotation is wrong and there's a good chance another part of the codebase could mess up when expecting a list.

[Dreamsorcerer]

OK, I see the root problem (which confirms to me that this is not being checked by mypy, as it should have errored here).

The method is defined on the base class, but it's not an abstract method. So, if inherited (or instantiated directly), this returns None, even though the annotation says it returns a list (which is a clear type error that would be caught by any type checker):

codecov-cli/codecov_cli/helpers/versioning_systems.py

Lines 23 to 24 in 63bd5f2

	def list_relevant_files(self, directory: t.Optional[Path] = None) -> t.List[str]:
	pass

I think the correct fix is to either replace that pass with a return [], so it actually does the correct behaviour, or to turn those methods into abstract methods and require the subclass to define them correctly.

[joseph-sentry]

Thanks for the feedback, I've implemented the ABC / abstractmethod suggestion.

[thomasrockhu-codecov]

Suggested change

	f"Found {len(network) if network else None} network files to report, ({len(unfiltered_network) if network else None} without filtering)"
	f"Found {len(network) if network else 0} network files to report, ({len(unfiltered_network) if network else 0} without filtering)"