Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1877943002:
Add unit test for providing empty list of detail items (Closed)
|
Created:
4 years, 8 months ago by rwlbuis Modified:
4 years, 8 months ago Reviewers:
please use gerrit instead CC:
chromium-reviews, blink-reviews, haraken Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd unit test for providing empty list of detail items
Add unit test for testing that an exception is thrown for an empty list
of detail items, this is different from not providing detail items
at all (ItemListRequired).
BUG=587995
Committed: https://crrev.com/beabbc4259fe54a818c1e824768495e8e8663406
Cr-Commit-Position: refs/heads/master@{#386569}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address review comment #Messages
Total messages: 14 (7 generated)
Description was changed from ========== Empty details items list BUG=587995 ========== to ========== Add unit test for providing empty list of detail items Add unit test for providing empty list of detail items, this is different from not providing detail items at all (ItemListRequired). BUG=587995 ==========
Description was changed from ========== Add unit test for providing empty list of detail items Add unit test for providing empty list of detail items, this is different from not providing detail items at all (ItemListRequired). BUG=587995 ========== to ========== Add unit test for providing empty list of detail items Add unit test for providing empty list of detail items, this is different from not providing detail items at all (ItemListRequired). BUG=587995 ==========
Description was changed from ========== Add unit test for providing empty list of detail items Add unit test for providing empty list of detail items, this is different from not providing detail items at all (ItemListRequired). BUG=587995 ========== to ========== Add unit test for providing empty list of detail items Add unit test for testing that an exception is thrown for an empty list of detail items, this is different from not providing detail items at all (ItemListRequired). BUG=587995 ==========
rob.buis@samsung.com changed reviewers: + rouslan@chromium.org
PTAL. I don't think we tested this before.
https://codereview.chromium.org/1877943002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/payments/PaymentRequestTest.cpp (right): https://codereview.chromium.org/1877943002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/payments/PaymentRequestTest.cpp:81: PaymentRequest::create(getScriptState(), Vector<String>(1, "foo"), buildPaymentDetailsForTest(PaymentTestDetailNoItems), getExceptionState()); I would prefer this to be: ------------------------------------------------- PaymentDetails details; details.setItems(HeapVector<PaymentItem>()); PaymentRequest::create(getScriptState(), Vector<String>(1, "foo"), details, getExceptionState()); ------------------------------------------------- Then you don't need changes in PaymentDetailsTestHelper.cpp and PaymentDetailsTestHelper.h.
PTAL. https://codereview.chromium.org/1877943002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/payments/PaymentRequestTest.cpp (right): https://codereview.chromium.org/1877943002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/payments/PaymentRequestTest.cpp:81: PaymentRequest::create(getScriptState(), Vector<String>(1, "foo"), buildPaymentDetailsForTest(PaymentTestDetailNoItems), getExceptionState()); On 2016/04/11 21:46:36, Rouslan wrote: > I would prefer this to be: > ------------------------------------------------- > PaymentDetails details; > details.setItems(HeapVector<PaymentItem>()); > > PaymentRequest::create(getScriptState(), Vector<String>(1, "foo"), details, > getExceptionState()); > ------------------------------------------------- > Then you don't need changes in PaymentDetailsTestHelper.cpp and > PaymentDetailsTestHelper.h. Good idea, done :)
The CQ bit was checked by rouslan@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1877943002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1877943002/20001
Message was sent while issue was closed.
Description was changed from ========== Add unit test for providing empty list of detail items Add unit test for testing that an exception is thrown for an empty list of detail items, this is different from not providing detail items at all (ItemListRequired). BUG=587995 ========== to ========== Add unit test for providing empty list of detail items Add unit test for testing that an exception is thrown for an empty list of detail items, this is different from not providing detail items at all (ItemListRequired). BUG=587995 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add unit test for providing empty list of detail items Add unit test for testing that an exception is thrown for an empty list of detail items, this is different from not providing detail items at all (ItemListRequired). BUG=587995 ========== to ========== Add unit test for providing empty list of detail items Add unit test for testing that an exception is thrown for an empty list of detail items, this is different from not providing detail items at all (ItemListRequired). BUG=587995 Committed: https://crrev.com/beabbc4259fe54a818c1e824768495e8e8663406 Cr-Commit-Position: refs/heads/master@{#386569} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/beabbc4259fe54a818c1e824768495e8e8663406 Cr-Commit-Position: refs/heads/master@{#386569} |
