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

Update Deserializes.php - Add DateTime format #744

Closed
wants to merge 1 commit into from

Conversation

simonsolutions
Copy link
Contributor

Add additional DateTime Format returned from Amazon ProductFeeV0 API

Add additional DateTime Format returned from Amazon ProductFeeV0 API
@jlevers
Copy link
Owner

jlevers commented Jun 25, 2024

Thanks! Would you mind adding a test supporting this?

@simonsolutions
Copy link
Contributor Author

A test should be in PR #743 ? It seems already been added there by someone else

@jlevers
Copy link
Owner

jlevers commented Jun 26, 2024

That PR had some issues, because it had code included from another PR. If you wouldn't mind writing a test that uses the endpoint where you encountered the error, that would be great!

@jlevers
Copy link
Owner

jlevers commented Jul 9, 2024

@simonsolutions when I asked you to combine #752, I just meant you could copy the code from that PR to this one. I'm going to assume you'll do that, and write my comments on that PR's code here.

I missed that you hadn't changed which request was being mocked – it looks like you copied and pasted the previous test without changing the mocks.

Since the API credentials supplied to the $connector in your test are invalid, if you ran that test it would give you an error. You'd need to mock the request/response that you're actually testing – in this case, GetMyFeesEstimateForSKU. However, that's not actaully necessary in this case. As I said in my comments in the other PR, check out this test for how you might go about testing this: https://github.com/jlevers/selling-partner-api/blob/main/tests/Seller/ProductFeesV0/Responses/GetMyFeesEstimateResponseTest.php.

Also, as I said in the other PR, please move the test to tests/Seller/ProductFeesV0/Responses/GetMyFeesEstimateResponseTest.php.

In general, please do not push tests (or other code) that you haven't actually run.

@simonsolutions
Copy link
Contributor Author

Sorry I'm confused what you now exactly want. I think it would be easier and with less misunderstanding if you put the code in the right place?!

@jlevers jlevers closed this Jul 9, 2024
@simonsolutions simonsolutions deleted the patch-1 branch August 5, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants