Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix AbstractTestApi::mockRequest assertions #150

Conversation

pdelre
Copy link
Contributor

@pdelre pdelre commented Oct 1, 2023

This addresses the issues in AbstractTestApi::mockRequest assertions and resolves the missed issues with many unit tests. Primarily the tests themselves have issues where they pass in an array but mockRequest asserts against a json string. Additionally when a $params['response'] was unset, AbstractTestApi::mockRequest would return null which was causing PHP Notices that the GitHub Actions configuration was hiding.

There was one issue with AbstractApi::_delete that was detected with the fixed assertions where $params was being passed to Crowdin::apiRequest without being wrapped into a request $options.

Fixes #149

@pdelre pdelre changed the title Fix AbstractTestApi::mockRequest assertions fix: Fix AbstractTestApi::mockRequest assertions Oct 1, 2023
@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #150 (dab053d) into master (a53a35f) will increase coverage by 0.01%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #150      +/-   ##
============================================
+ Coverage     90.41%   90.42%   +0.01%     
- Complexity     1181     1182       +1     
============================================
  Files           107      107              
  Lines          3076     3079       +3     
============================================
+ Hits           2781     2784       +3     
  Misses          295      295              
Files Coverage Δ
src/CrowdinApiClient/Api/AbstractApi.php 78.17% <100.00%> (+0.78%) ⬆️

@andrii-bodnar
Copy link
Member

@pdelre awesome, thanks a lot for the contribution! 🚀

@andrii-bodnar andrii-bodnar merged commit cfb9a68 into crowdin:master Oct 3, 2023
8 of 9 checks passed
@andrii-bodnar andrii-bodnar added hacktoberfest This issue welcomes contributions for Hacktoberfest hacktoberfest-accepted labels Oct 3, 2023
@pdelre pdelre deleted the pdelre_pr_issue-149-fix-mockRequest-assertions branch October 3, 2023 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest This issue welcomes contributions for Hacktoberfest hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AbstractTestApi::mockRequest fails to assert body & header
2 participants