Skip to content

Conversation

@uds5501
Copy link
Contributor

@uds5501 uds5501 commented Aug 22, 2019

Fixes #3468

Short description of what this resolves:

Add integration tests for paytm modals\

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@uds5501 uds5501 added the tests label Aug 22, 2019
@uds5501 uds5501 force-pushed the test-modals branch 2 times, most recently from bd0db75 to c9a7a09 Compare August 22, 2019 20:53
@fossasia fossasia deleted a comment Aug 22, 2019
Copy link
Member

@Anupam-dagar Anupam-dagar left a comment

Choose a reason for hiding this comment

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

Your new test is failing, please fix.

Copy link
Member

@shreyanshdwivedi shreyanshdwivedi left a comment

Choose a reason for hiding this comment

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

LGTM now 👍

@uds5501
Copy link
Contributor Author

uds5501 commented Aug 24, 2019

@kushthedude @mrsaicharan1 Please review

@uds5501
Copy link
Contributor Author

uds5501 commented Aug 24, 2019

This needs to be updated once #3476 is updated

Anupam-dagar
Anupam-dagar previously approved these changes Aug 28, 2019
Copy link
Member

@Anupam-dagar Anupam-dagar left a comment

Choose a reason for hiding this comment

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

I guess it can be merged now since the dependent PR is merged.

@uds5501
Copy link
Contributor Author

uds5501 commented Aug 28, 2019

@Anupam-dagar I will update this PR today itself and ping you to review.

@fossasia fossasia deleted a comment Aug 28, 2019
@uds5501 uds5501 dismissed stale reviews from Anupam-dagar and shreyanshdwivedi via badfdb3 August 28, 2019 20:17
@fossasia fossasia deleted a comment Aug 28, 2019
@uds5501
Copy link
Contributor Author

uds5501 commented Aug 28, 2019

@uds5501 uds5501 requested a review from abhinavk96 August 29, 2019 18:07
@fossasia fossasia deleted a comment Aug 29, 2019
Copy link
Member

@shreyanshdwivedi shreyanshdwivedi left a comment

Choose a reason for hiding this comment

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

Travis was failing. I've restarted the build

@fossasia fossasia deleted a comment Aug 30, 2019
Copy link
Member

@shreyanshdwivedi shreyanshdwivedi left a comment

Choose a reason for hiding this comment

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

The tests look fine 👍

'isOpen' : false,
'currency' : 'USD',
'amount' : 100,
'verifyOtp' : () => {}
Copy link
Member

Choose a reason for hiding this comment

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

This looks fine but what does this test exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the current state, it checks rendering of the modal without actually hitting the API (gotta save the API calls xD )

@abhinavk96 abhinavk96 merged commit 9968963 into fossasia:development Aug 31, 2019
@fossasia fossasia deleted a comment Aug 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add integration tests for new modals

5 participants