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

Test beta env #370

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Test beta env #370

wants to merge 10 commits into from

Conversation

ankitdas13
Copy link
Contributor

Created Test Razorpay instance for test url

@codecov-commenter
Copy link

Codecov Report

Merging #370 (5170381) into master (b82373d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #370   +/-   ##
=======================================
  Coverage   91.41%   91.41%           
=======================================
  Files          27       27           
  Lines        1072     1072           
=======================================
  Hits          980      980           
  Misses         92       92           

Choose a reason for hiding this comment

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

Can we rename the folder to e2e ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have renamed the folder to e2e

class RazorpayTest extends Razorpay {
constructor(options) {
super(options)
this.api.rq = request.defaults({
Copy link

Choose a reason for hiding this comment

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

Can we pass hostUrl as options in constructor of Razorpay itself ?
By default, it would be api.razorpay.com else it can be https://api-web.dev.razorpay.in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added hostUrl in options

Choose a reason for hiding this comment

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

I didn't mean adding it in RazorpayTest but in Razorpay class. In that case, we don't need RazorpayTest itself. We can directly use Razorpay by passing hostUrl.
Sorry for this confusion.

Choose a reason for hiding this comment

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

Just realised that adding it in class Razorpay would have other problems since that is a public API. Let's keep this as such for now.

e2e/razorpay.js Outdated
const Razorpay = require("../dist/razorpay");
let request = require('request-promise');

class RazorpayTest extends Razorpay {

Choose a reason for hiding this comment

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

Let's rename this Class to RazorpayBeta

Copy link

@1995navinkumar 1995navinkumar left a comment

Choose a reason for hiding this comment

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

Approving with one comment

'use strict';

const Razorpay = require("../dist/razorpay");
let request = require('request-promise');

Choose a reason for hiding this comment

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

question: we can use const here?

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.

None yet

4 participants