Skip to content

Conversation

@bourgeoa
Copy link
Contributor

@SharonStrats

I think I have something to work with if we can begin to add tests
first test in test/chat/keys.test.ts is failing with

crypto.getRandomValues must be defined

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

(editorial) minor fix to a comment

const res = serializeMsg(msg)
expect(res).toEqual(JSON.stringify(msg))
})
it.skip('sign and verifySignature', () => {
Copy link
Contributor

@SharonStrats SharonStrats May 17, 2023

Choose a reason for hiding this comment

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

@angelo-v I'm having trouble with this test. I've tried several different things. I was wondering if you can tell just by looking what I'm doing wrong. Here is the error. I've tried to add a test using try/catch but that also didn't seem to work.
[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "undefined".] { code: 'ERR_UNHANDLED_REJECTION'

Copy link
Contributor Author

@bourgeoa bourgeoa May 17, 2023

Choose a reason for hiding this comment

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

I do not have your issue but TypeError: Expected input type is Uint8Array (got object)
This seems a jest/jsdom issue with a solution proposed in paralleldrive/cuid2#44 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @bourgeoa I will check that out.

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Minor grammar, punctuation, etc.

Comment on lines 38 to 40
##### Any

##### Each
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
##### Any
##### Each
##### `Any`
##### `Each`

Copy link
Contributor

Choose a reason for hiding this comment

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

@TallTed thank you for all the corrections.

contentType: 'text/turtle'
})
} catch (err) {
if (err?.response?.status !== 404) { throw new Error(err) }
Copy link
Contributor

Choose a reason for hiding this comment

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

@bourgeoa I'm wondering, if it errors with a 404, should we create the resource in that case?

And otherwise if it errors would it be because the user is not logged in? If so I will mock that data for testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PUT should never error with 404 so we should remove the if
The reason for failing could be 401 or 403 (not authenticated or not authorized)

import * as ns from '../../ns'
import { NamedNode } from 'rdflib'

export const getPodRoot = async (webId: NamedNode) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@bourgeoa bourgeoa May 18, 2023

Choose a reason for hiding this comment

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

Good find. I did not knew this function.
I think :

  • keep for know the one we have which is dealing with more cases : multiple storages or no storage in WebID
  • make the one we have compatible by returning a NamedNode
  • create an issue/PR in solid-logic to improve the getPodRoot()

SharonStrats and others added 9 commits May 18, 2023 09:32
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
@bourgeoa bourgeoa merged commit ddcbbf4 into main May 25, 2023
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.

4 participants