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

API. No auction response #123

Merged
merged 5 commits into from
Jul 30, 2019

Conversation

yoalex5
Copy link
Collaborator

@yoalex5 yoalex5 commented Jul 3, 2019

GitHub issue #122

  1. PREBID_SERVER_ERROR was made as default error
  2. Unit tests were modified

@yoalex5 yoalex5 changed the title No auction response API. No auction response Jul 3, 2019
@yoalex5 yoalex5 mentioned this pull request Jul 4, 2019
avohraa
avohraa previously approved these changes Jul 15, 2019
…tion_response_error

# Conflicts:
#	PrebidMobile/API1.0/src/test/java/org/prebid/mobile/PrebidServerAdapterTest.java
@yoalex5
Copy link
Collaborator Author

yoalex5 commented Jul 26, 2019

@avohraa, could you please approve this PR?
I have just merged the master to the current branch

@yoalex5
Copy link
Collaborator Author

yoalex5 commented Jul 29, 2019

@bszekely1, will you please review this PR. All the changes were approved by @avohraa, I just merged master into this branch. After that I will be unblocked

ArgumentCaptor<String> uuidAC = ArgumentCaptor.forClass(String.class);

verify(mockListener).onDemandFailed(resultCodeAC.capture(), uuidAC.capture());
assertThat(resultCodeAC.getValue(), Matchers.either(Matchers.is(ResultCode.INVALID_ACCOUNT_ID)).or(Matchers.is(ResultCode.PREBID_SERVER_ERROR)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the cases that enforce us to assert two ResultCode?

  • INVALID_ACCOUNT_ID
  • PREBID_SERVER_ERROR

This test case is for the INVALID_ACCOUNT_ID, then why should it pass if onDemandFailed is obtained with other ResultCode (PREBID_SERVER_ERROR in this case)

Also, can we mock the response instead? Something like this:
server.enqueue(new MockResponse().setResponseCode(400).setBody("Invalid request format: No stored request found for id: 1001_INVALID_ACCOUNT_ID"));

ArgumentCaptor<String> uuidAC = ArgumentCaptor.forClass(String.class);

verify(mockListener).onDemandFailed(resultCodeAC.capture(), uuidAC.capture());
assertThat(resultCodeAC.getValue(), Matchers.either(Matchers.is(ResultCode.INVALID_CONFIG_ID)).or(Matchers.is(ResultCode.PREBID_SERVER_ERROR)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make similar changes for INVALID_CONFIG_ID as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avohraa, I have updated the PR

@yoalex5
Copy link
Collaborator Author

yoalex5 commented Jul 30, 2019

@avohraa, could you please review these changes

if (!successfulMockServerStarted) {
fail("Mock server was not started");
}
server.enqueue(new MockResponse().setResponseCode(400).setBody("Invalid request format: No stored imp found for id: 1001-1_INVALID_CONFIG_ID"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for reiterating on this, but can we have that response body from the MockPrebidServerResponses?

Something like: MockPrebidServerResponses.invalidConfigIdFromRubicon.

if (!successfulMockServerStarted) {
fail("Mock server was not started");
}
server.enqueue(new MockResponse().setResponseCode(400).setBody("Invalid request format: No stored request found for id: 1001_INVALID_ACCOUNT_ID"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Invalid Account Id as well.

@yoalex5
Copy link
Collaborator Author

yoalex5 commented Jul 30, 2019

@avohraa, done

Copy link
Collaborator

@avohraa avohraa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

@yoalex5 yoalex5 merged commit eb18f5f into prebid:master Jul 30, 2019
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