dtinit / data-transfer-project

The Data Transfer Project makes it easy for platforms to build interoperable user data portability features. We are establishing a common framework, including data models and protocols, to enable direct transfer of data both into and out of participating online service providers.
https://dtinit.org/docs/dtp-what-is-it
Apache License 2.0
3.57k stars 482 forks source link

techdebt: backfill missing unit tests for GooglePhotosInterface.java #1158

Open jzacsh opened 2 years ago

jzacsh commented 2 years ago

@MewX ran into this at https://github.com/google/data-transfer-project/pull/1155#issuecomment-1240244384

mahikaajain commented 1 year ago

Hey, is this issue still open? If yes, can I work on this?

anand007s commented 1 year ago

please assign it to me...

FilippoULIVO commented 1 year ago

Hey, I noticed that most methods on the class depend on private method makeGetRequest in order to mock it I would need to make it visible but I don't want to break encapsulation. I noticed there is also a makePostRequest which is public and is used for mocked responses in GooglePhotosImporterTest. If it's okay to change the access modifier on the method I can complete unittesting the methods dependent on makeGetRequest.

@jzacsh

MewX commented 1 year ago

Would package-private sufficient for the tests? If so, usually you could mark these methods package-private and annotate them with @VisibleForTesting.

FilippoULIVO commented 1 year ago

yes ok it works. currentlyt working on this

FilippoULIVO commented 1 year ago

Hey ,sorry for the noob question, I can't find this info. I'm trying to make a first commit with most of the test cases but I'm not able to push to origin, I'm trying git push -u origin test-importer-interface and receiving 403 error with message remote: Permission to google/data-transfer-project.git denied to FilippoULIVO I just submitted my CLA agrrement, but i'm not sure it's related to that. could you help?

wanted to make a first commits also to check if I could pass all the different style checks

FilippoULIVO commented 1 year ago

I completed the test coverage for the class. Btw @MewX as I was sayngmakePostRequest is public, I didn't write test for it because I belive it should be made package-private just as the makeGetRequest ,having it public defeats the purpose of the interface, from IntelliJ I can see it's only used in tests as mock. I would need some guidance on the Pull request process for submitting my contribute

jzacsh commented 1 year ago

I'm trying to make a first commit with most of the test cases but I'm not able to push to origin, I'm trying git push -u origin test-importer-interface and receiving 403 error with message remote: Permission to google/data-transfer-project.git denied to FilippoULIVO

I believe that's because you're accidentally trying to develop directly on this repo, but you instead want to maintain your branch on your own repo (both in draft state and when you're ready to open a PR for it). You can get your own repo by clicking the "fork" button (or visiting https://github.com/google/data-transfer-project/fork).

maybe-helpful guessing on my part here, but please read the docs and run at your own risk (backup your work, etc) Since you already have a working copy of the repo in your terminal, you can tell that copy tell about your new fork by adding a new git "remote" like so: ```sh $ git remote -v # pay attention to the output from this command; note how it will change later (you'll probably only have "origin" lines right now)... $ git remote add fork git@github.com:FilippoULIVO/data-transfer-project.git $ git remote -v # now "fork" should be listed to ``` I'm guessing there that you have SSH configured, but [if not you may want to `set-url` to HTTPS instead](https://docs.github.com/en/get-started/getting-started-with-git/managing-remote-repositories#changing-a-remote-repositorys-url) before running this command: ``` $ git push fork test-importer-interface ``` Once that's all done your branch "test-importer-interface" will be visible in the github UI under your account (from which a PR can be opened when you're ready)
FilippoULIVO commented 1 year ago

Please review?

FilippoULIVO commented 1 year ago

Hey, if this pull request is okay I'm happy to keep contributing to the project @jzacsh

FilippoULIVO commented 1 year ago

sorry for the repeated trial and error. I don't understand if my pull request is well formulated or if I'm doing something wrong

FilippoULIVO commented 1 year ago

hey @jzacsh @MewX is this issue still relevant? could you review my merge request please? thanks