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

[PinPayments] Implemented test cases for authorize #139

Open
wants to merge 12 commits into
base: pinpay
Choose a base branch
from

Conversation

ravirocx
Copy link

Implemented test cases for good_card, bad_card and with card_token

@ravirocx ravirocx changed the title Implemented test cases for authorize [PinPayments] Implemented test cases for authorize Mar 28, 2018
@oyeb oyeb self-requested a review April 9, 2018 08:10
Copy link
Contributor

@oyeb oyeb left a comment

Choose a reason for hiding this comment

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

Please write some meaningful tests to validate presence and correctness of required fields in opts.
Please rebase your branch!


alias Gringotts.Gateways.PinPayments, as: Gateway

#@moduletag :integration
Copy link
Contributor

@oyeb oyeb Apr 9, 2018

Choose a reason for hiding this comment

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

Please uncomment this line.

@@ -0,0 +1,84 @@
defmodule Gringotts.Integration.Gateways.PinpaymentsTest do
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in module name.

description: "hello",
email: "[email protected]",
ip_address: "1.1.1.1",
config: %{apiKey: "c4nxgznanW4XZUaEQhxS6g", pass: ""}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is :pass in config, to best of my memory we removed it from the required_config.

ip_address: "1.1.1.1",
config: %{apiKey: "c4nxgznanW4XZUaEQhxS6g", pass: ""}

] ++ [address: @add]
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't concat lists like this.

assert response.status_code == 400
end

test "[authorize] with bad CreditCard 2" do
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this pointless test case, and @badcard2

first_name: "Harry",
last_name: "Potter",
number: "4200000000000000",
year: 2019,
Copy link
Contributor

Choose a reason for hiding this comment

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

This will become a bad card in a year. Please use a larger expiry year.

email: "[email protected]",
ip_address: "1.1.1.1",
config: %{apiKey: "c4nxgznanW4XZUaEQhxS6g"}
] ++ [address: @add]
Copy link
Contributor

Choose a reason for hiding this comment

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

I've already pointed out in an earlier comment that you should not concat lists like this. The second list has only one element, moreover, the first list is being defined right there. You can simply move address: @add inside.

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