crowdin / crowdin-api-client-dotnet

.NET client library for Crowdin API
https://www.nuget.org/packages/Crowdin.Api/
MIT License
52 stars 27 forks source link

chore(test): Refactor Core directory in the Crowdin.Api.Tests #274

Closed EvilKot closed 2 weeks ago

EvilKot commented 1 month ago

Closes #272

Reaming folder with test utilities to Testing/ instead of /Core to not to conflict with folder structure defined in /src.

Should be good enough. Alternatively all tests could be moved into nested directory to avoid further folder conflicts, or even all test utilities and resources could be moved to a new project.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 63.46%. Comparing base (f45d418) to head (8785048). Report is 115 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #274 +/- ## =========================================== + Coverage 50.38% 63.46% +13.08% =========================================== Files 277 400 +123 Lines 3688 6096 +2408 Branches 0 503 +503 =========================================== + Hits 1858 3868 +2010 - Misses 1830 2189 +359 - Partials 0 39 +39 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

andrii-bodnar commented 1 month ago

@EvilKot, thank you! Passing it over to @innomaxx for review as well.

innomaxx commented 1 month ago

@andrii-bodnar @EvilKot nice work, but I suggest a slightly different approach.

To refactor the namespaces, we can make the following changes:

  1. Rename the project Crowdin.Api.Tests to Crowdin.Api.UnitTesting (it looks better IMO).
  2. Move all tests to Crowdin.Api.UnitTesting.Tests.{ApiSectionName} folders Core serialization and other tests place in root of Crowdin.Api.UnitTesting.Tests
  3. Move all resources to Crowdin.Api.UnitTesting.Resources folder.
  4. Move helper and utility classes to Crowdin.Api.UnitTesting folder.
EvilKot commented 1 month ago

image

EvilKot commented 2 weeks ago

Hey, sorry for a long reply.

It seems i've messed up a bit with commit history trying to merge latest changes 😬 Also not quite sure why files such Readme.md and two files in Crowdin.Api.Translations are listed in the changes. I've created new PR #278 with all the changes applied from scratch in a single commit, just in case.

1-4 Done 5 - files under Crowdin.Api.UnitTesting, Crowdin.Api.UnitTesting.Resources, Crowdin.Api.UnitTesting.Tests looks fine, Crowdin.Api.UnitTesting.Tests.{ApiSectionName} and below, like Crowdin.Api.UnitTesting.Tests.Webhooks.Organization partially checked, renaming was done by IDE.

innomaxx commented 2 weeks ago

Answered in #278