ADD: Tests for Google Drive and AWS Connection Methods

[cs7-shrey]

Description

This PR adds tests for Google Drive and AWS connection methods including tests for transfers, TUI and unit tests for certain functions.

Checklist:

• The code has been tested locally
• Tests have been added to cover all new functionality
• The documentation has been updated to reflect any changes
• The code has been formatted with pre-commit

[cs7-shrey]

Hey @JoeZiminski, for some reason, the tests are still failing due to invalid credentials. Can you check the github secrets again and make sure they are correct and their variable names are the same as that in code_test_and_deploy.yml (probably this naming issue is causing tests to fail)?

[JoeZiminski]

Thanks @cs7-shrey I'll look into this now

[cs7-shrey]

Hey @JoeZiminski, I couldn't understand this part of code from #572

def check_if_aws_bucket_exists(cfg: Configs) -> bool:


    if bucket_name != "" and bucket_name not in names:  # TODO: CHECK
        return False

    return True

Why do we return True in case a bucket name is ""? If the user provided no bucket shouldn't we be returning False?

[JoeZiminski]

Hey @cs7-shrey sorry I just hacked that together and was operating under a bad assumption. For gdrive it is possible to set central_path to None in case the folder root id is the folder the user wants to store the data in. I thought there was a similar mechanism with AWS in that you could just dump data directly in the root bucket. But now I see that this is not possible for AWS, there is no concept of a root bucket and central_path must point to the bucket name.

In this case, that code and the changes to get_aws_bucket_name can be completely reverted. Also, previously I set it up so None was a valid argument for aws (as well as gdrive). Let me know when you are finished here, prior to full review I will revert this in this PR. Or, if its easier I can revert on add_gdrive_aws_remote and you can rebase this onto that branch.

[cs7-shrey]

Cool @JoeZiminski! I'll revert changes to that function. Just to be on the same page, you're talking about reverting the central path being None in AWS right? If so, I think you can push directly to add_gdrive_aws_remote and I can rebase thereafter.

[JoeZiminski]

Hi @cs7-shrey that's what I meant (so central_path=None is only valid for gdrive. Okay great, will do that now!

[JoeZiminski]

Hi @cs7-shrey sorry related to your question about the long test-times. Currently we run the tests under two schedules, the one of the PRs runs only the first and latest python, while another runs weekly on all version

python-version: ${{ fromJson(github.event_name == 'schedule' && '["3.9", "3.10", "3.11", "3.12", "3.13"]' || '["3.9", "3.13"]') }}

We could extend this to write an environment variable like QUICK_TESTS or something and have only a subset of the parameterisations (or set up tests, whatever it is slow) run on the PRs. Then for the weekly tests we can run everything.

[adamltyson]

Could I suggest that all the tests should be run every time? It might be worth cutting down the parametrisation and/or trying to speed the tests up, but in my experience, tests that aren't run may as well not exist. It's really easy to ignore scheduled tests, or ones that need to be run manually. The GH actions test matrix runs in parallel anyway.

[cs7-shrey]

Hey @JoeZiminski @adamltyson, the google drive tests are indeed slow (as they were on my device) but on github actions they are significantly faster. Even then, the total time for which the tests are run will eventually blow up as this gets merged into main and more and more tests keep getting added. So, in my opinion it might be worth thinking a strategy for that (not much of a concern right now but more so in the future),

[cs7-shrey]

Also, @JoeZiminski, I've written all the tests I could think of. If you think of adding any more tests, do let me know. If this seems fine, I'll proceed to write docstrings for tests and functions written in this PR.

[JoeZiminski]

Awesome, thanks @cs7-shrey! I will review this ASAP. Good point @adamltyson we can think of ways to speed up these tests where possible and explore any alternatives to splitting the tests in that way.

[JoeZiminski]

Hey @cs7-shrey this is excellent, I don't think it could be any neater or more centralized. I've added a few suggestions here and there, very minor stuff mostly around consistency with existing code conventions. After taking a look at these suggestions this will be good to go!

[JoeZiminski]

Awesome! thanks @cs7-shrey The SonarCloud is failing for a non-reason, are you able to see it? Basically in the google drives / aws tests TestTuiGdriveConfigs and the similar AWS test there is quite a lot of similar code and it is complaining about duplication. But, these are tests and I think it is more readable as it is, so there is no need to change it. Unfortunately I can't override it but will just merge, it won't make a difference going forward.