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

fix: use custom document loader in jsonld.frame #1119

Conversation

karimStekelenburg
Copy link
Contributor

We use a custom document loader for tests that use local files instead of downloading them from the web. Inside this custom document loader, we're calling jsonld.frame(), but didn't pass the custom document loader. As a result, documents were being fetched from the web during tests, which made the tests fail when there is no internet connection.

This minor fix passes the custom document loader to the jsonld.frame() function and makes sure only local files are used.

Fixes #1111

Signed-off-by: Karim Stekelenburg [email protected]

@karimStekelenburg karimStekelenburg requested a review from a team as a code owner November 24, 2022 13:50
Copy link
Contributor

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

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

That's the longest branch name I've ever seen :)

@TimoGlastra TimoGlastra enabled auto-merge (squash) November 24, 2022 14:04
@genaris
Copy link
Contributor

genaris commented Nov 24, 2022

That's the longest branch name I've ever seen :)

Testing the boundaries of git! 😛

@karimStekelenburg
Copy link
Contributor Author

Haha, it's what happens if you let vscode create a branch based on an issue 🤣

@karimStekelenburg karimStekelenburg force-pushed the karimStekelenburg/issueW3cCredentialService-Custom-Document-Loader-does-not-work-when-no-Internet-->-e2e-Tests-Fail branch from 955aefb to 2dfe0ae Compare December 5, 2022 18:00
@@ -51,6 +51,7 @@ export const BBS_V1 = {
'@protected': true,
id: '@id',
type: '@type',

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Signed-off-by: Karim Stekelenburg <[email protected]>
@genaris
Copy link
Contributor

genaris commented Dec 6, 2022

@blu3beri is it OK to merge? (I've seen you disabled auto-merge but don't know if it's for an specific reason)

@berendsliedrecht
Copy link
Contributor

@blu3beri is it OK to merge? (I've seen you disabled auto-merge but don't know if it's for an specific reason)

I think @karimStekelenburg wants to add something to an error message still. I will merge it when he adds it :).

@TimoGlastra
Copy link
Contributor

@karimStekelenburg is this ready?

…Custom-Document-Loader-does-not-work-when-no-Internet-->-e2e-Tests-Fail
@TimoGlastra TimoGlastra enabled auto-merge (squash) December 12, 2022 08:36
@codecov-commenter
Copy link

Codecov Report

Merging #1119 (b7a0899) into main (3c040b6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1119   +/-   ##
=======================================
  Coverage   88.32%   88.32%           
=======================================
  Files         706      706           
  Lines       16431    16431           
  Branches     2664     2664           
=======================================
  Hits        14513    14513           
  Misses       1911     1911           
  Partials        7        7           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@TimoGlastra TimoGlastra merged commit 36d4656 into openwallet-foundation:main Dec 12, 2022
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.

W3cCredentialService Custom Document Loader does not work when no Internet -> e2e Tests Fail
5 participants