Improve tests #141
Improve tests #141
Conversation
| } | ||
| expect(metadata).to.be.undefined; | ||
| }); | ||
|
|
There was a problem hiding this comment.
Additional test coverage is welcome, but what does this have to with the change from url.parse() to new URL()`?
There was a problem hiding this comment.
Its a legacy API
The Node.sj documentation said Use the WHATWG URL API instead.
The possible argument tu use url.parse are :
Faster than the alternative WHATWG URL parser.- it is used intestfiles so I think it's not the purpose hereEasier to use with regards to relative URLs than the alternative WHATWG URL API.- thenew URLworks good in our case
There was a problem hiding this comment.
But I agree it can be split into 2 PR
There was a problem hiding this comment.
I'm not concerned about the performance difference in the URL parses. SAML would a small amount of traffic for most web-apps and URL parsing is probably not the bottleneck.
I'm OK with leaving this as one PR as long as @cjbarth agrees about accepting the additional test coverage.
There was a problem hiding this comment.
Okay great
I can also split the PR if needed 😃
|
For added context, the new In Node 11, Still, because I left a comment about the additional test coverage that I'm not sure relates. |
Its-Just-Nans
changed the title
Codecov Report
@@ Coverage Diff @@
## master #141 +/- ##
==========================================
+ Coverage 80.82% 81.06% +0.24%
==========================================
Files 13 13
Lines 829 829
Branches 242 242
==========================================
+ Hits 670 672 +2
+ Misses 72 71 -1
+ Partials 87 86 -1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
Description
url.parse()intonew URL()generateServiceProviderMetadata throw errorChecklist: