intuit / oauth-jsclient

Intuit's NodeJS OAuth client provides a set of methods to make it easier to work with OAuth2.0 and Open ID
https://developer.intuit.com/
Apache License 2.0
121 stars 156 forks source link

Improve code coverage #39

Closed abisalehalliprasan closed 4 years ago

abisalehalliprasan commented 4 years ago

Goal : To improve the test coverage.

Ref : Steps to test the coverage locally: https://github.com/intuit/oauth-jsclient#steps

edgarsherman commented 4 years ago

I'd be happy to take a look at this for Hacktoberfest

edgarsherman commented 4 years ago

@abisalehalliprasan I added a PR for one file and am working on OAuthClient.js. I started at the bottom working my way up and came across createError. This function seems odd and I'm not sure of the correct behavior.

OAuthClient.prototype.createError = function createError(e, authResponse) {
  if (!authResponse || authResponse.body === '') {
    e.error = e.originalMessage || '';
    e.authResponse = authResponse || '';
    e.intuit_tid = authResponse.headers().intuit_tid || '';
    e.originalMessage = authResponse.response.statusText || '';
    e.error = authResponse.response.statusText || '';
    e.error_description = authResponse.response.statusText || '';
    return e;
  }

  e.authResponse = authResponse || null;
  e.originalMessage = e.message;
  e.error = ('error' in authResponse.getJson() ? authResponse.getJson().error : '');
  e.error_description = ('error_description' in authResponse.getJson() ? authResponse.getJson().error_description : '');
  e.intuit_tid = authResponse.headers().intuit_tid;

  return e;
};

In the first IF block, we check if AuthResponse is null (empty, undefined) but then start to use functions properties of it. Also, e.error is redefined.
I'd be happy to clean it up as part of writing tests for it - does this look appropriate?

OAuthClient.prototype.createError = function createError(e, authResponse) {
  if (!authResponse || authResponse.body === '') {
    e.error = (authResponse && authResponse.response.statusText) || e.message || '';
    e.authResponse = authResponse || '';
    e.intuit_tid = (authResponse && authResponse.headers().intuit_tid) || '';
    e.originalMessage = e.message || '';
    e.error_description = (authResponse && authResponse.response.statusText) || '';
    return e;
  }

  e.authResponse = authResponse;
  e.originalMessage = e.message;
  e.error = ('error' in authResponse.getJson() ? authResponse.getJson().error : '');
  e.error_description = ('error_description' in authResponse.getJson() ? authResponse.getJson().error_description : '');
  e.intuit_tid = authResponse.headers().intuit_tid;

  return e;
};
abisalehalliprasan commented 4 years ago

@edgarsherman : Looks good 👍

mandardeshpande commented 4 years ago

@abisalehalliprasan : is there still work on this left ? If so i can take it..

abisalehalliprasan commented 4 years ago

@mandardeshpande : You can split work if there are any files that @edgarsherman has not completed/worked on.

Since this is an Open Source Contribution, you should be free to submit a PR.