feat(screenshots): add the stringIds parameter support

[EvilKot]

Closes #256

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.38%. Comparing base (f45d418) to head (02a681e).
Report is 111 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #271       +/-   ##
===========================================
+ Coverage   50.38%   63.38%   +13.00%     
===========================================
  Files         277      399      +122     
  Lines        3688     6084     +2396     
  Branches        0      501      +501     
===========================================
+ Hits         1858     3856     +1998     
- Misses       1830     2189      +359     
- Partials        0       39       +39     

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

[EvilKot]

Hello, creating a PR in relation to #256 (comment)

I haven't found any tests related to InternalExtensions, except one checking orderBy string generation.
Also there's no tests checking Screenshots.ListScreenshots method (only this ones) so i have no reference/example how to add a new ones.

If it's a mandatory, I'd be really grateful for any help and tips how i could add them.
Thank you!

[andrii-bodnar]

Hello @EvilKot, thanks a lot for the contribution!

It would be nice to add tests for the InternalExtensions class and Screenshots.ListScreenshots method. For reference, I think you can use the ProjectsApiTests:ListProjects test case.

The testing approach requires some boilerplate code like resources (e.g. Projects.resx, Projects.Designer.cs). I'm not quite sure how this code is generated.

[EvilKot]

Thank you, @andrii-bodnar!

For Screenshots.ListScreenshots i've added a test based on example you provided. I however manually adjusted corresponsing .resx and .Designer.cs files.

For InternalExtensions i'm not sure i'm going in a right way and if it's actually worth to cover it with tests:

• Most of the class methods are already being tested indirectly.
• I placed my InternalExtensionsTest.cs file in tests/Crowdin.Api.Tests/Core directory, mirroring the placement of InternalExtensions.cs located in src/Crowdin.Api/Core, similar to how it's done for other tests. However Core directory inside Crowdin.Api.Tests project serves a bit different purpose and now only contains shared test utilities and .resx files. If the long term goal is to also cover everything in src/Crowdin.Api/Core with tests, then i'd suggest moving util and .resx files to another directory, hmmm, maybe something like tests/Crowdin.Api.Tests/TestUtils?
• Covered only query string generation methods.

One more thing - i'm a bit struggling with indentation settings used in this repo.
Originally my IDE removed whitespaces from blank lines and i not sure how to recover it and if it makes any difference.

🥹

[andrii-bodnar]

@EvilKot thank you!

i'd suggest moving util and .resx files to another directory, hmmm, maybe something like tests/Crowdin.Api.Tests/TestUtils

It definitely makes a lot of sense to move those resources and utilities to TestUtils.

One more thing - i'm a bit struggling with indentation settings used in this repo.
Originally my IDE removed whitespaces from blank lines and i not sure how to recover it and if it makes any difference.

I think it would be better to undo this change and avoid trailing spaces.

[EvilKot]

@andrii-bodnar, i'm afraid this PR will become super inflated if i move things into /TestUtils, since it will impact almost every file in tests project 😅

indentation updated 👍

[andrii-bodnar]

@EvilKot sure, it's definitely out of the scope of the current PR

[andrii-bodnar]

@EvilKot I just created a separate issue for that - #272

[andrii-bodnar]

Looks good to me, thanks @EvilKot!

[EvilKot]

Yay, thank you for walking me through!