Skip to content

Conversation

@AronBuzogany
Copy link
Collaborator

This PR fixes #21

@AronBuzogany AronBuzogany requested a review from pheyvaer August 18, 2023 07:05
@AronBuzogany AronBuzogany self-assigned this Aug 18, 2023
Copy link
Contributor

@pheyvaer pheyvaer left a comment

Choose a reason for hiding this comment

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

Is the "Testing" section of the README up to date?

@AronBuzogany
Copy link
Collaborator Author

Is the "Testing" section of the README up to date?

No, I have updated it in commit 1d852c1

@AronBuzogany AronBuzogany requested a review from pheyvaer August 18, 2023 08:42
@pheyvaer
Copy link
Contributor

Is the "Testing" section of the README up to date?

No, I have updated it in commit 1d852c1

Did you try every step yourself?

@AronBuzogany
Copy link
Collaborator Author

Did you try every step yourself?

Just did again, it works for me

@pheyvaer
Copy link
Contributor

pheyvaer commented Aug 18, 2023

Did you try every step yourself?

Just did again, it works for me

Weird, because when I do the second step with npm start I get this error

npm ERR! Missing script: "start"

@AronBuzogany
Copy link
Collaborator Author

Weird, because when I do the second step with npm start I get this error

Ah, yes, my bad, I have custom npm commands configured so I didn't notice.

You would probably get the same for npm test

I have updated it in commit 362d13e

Copy link
Contributor

@pheyvaer pheyvaer left a comment

Choose a reason for hiding this comment

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

"Log in and execute query on private data" and "Querying resource with "bad" cors header, though a proxy should work" doesn't work for me. I think it's related to the fact that when the app starts immediately the first query is executed. There is a PR #31 to fix this.

@AronBuzogany
Copy link
Collaborator Author

Strange, I just executed it locally and they all passed.

image

I tried it with cypress open instead of cypress run also, and it doesn't seem to be doing anything weird.

@pheyvaer
Copy link
Contributor

@AronBuzogany Can you merge main into the branch and then see if everything still works for you?

@AronBuzogany
Copy link
Collaborator Author

@AronBuzogany Can you merge main into the branch and then see if everything still works for you?

they still all succeed

image

@AronBuzogany AronBuzogany mentioned this pull request Aug 29, 2023
@pheyvaer
Copy link
Contributor

Ok. Can you remove all DBpedia data sources from the tests? It's best not to rely on remote resources for testing.

@AronBuzogany
Copy link
Collaborator Author

Yes, in that case I will just replace all the queries which don't query locally hosted resources

@AronBuzogany
Copy link
Collaborator Author

Ok. Can you remove all DBpedia data sources from the tests? It's best not to rely on remote resources for testing.

They are now all removed except for the ask query as it is not implemented yet #22

All tests still succeed

image

@pheyvaer
Copy link
Contributor

I see there is startBadCorsServer.mjs but it doesn't seem like we start it anywhere? Is that correct?

@AronBuzogany
Copy link
Collaborator Author

I see there is startBadCorsServer.mjs but it doesn't seem like we start it anywhere? Is that correct?

Correct, I haven't got there yet. It is now however in the README and a script in package.json

Should we do something about the number of processes running up? Or is it okay as is?

@AronBuzogany AronBuzogany requested a review from pheyvaer August 30, 2023 06:01
@AronBuzogany AronBuzogany merged commit b83d4b9 into main Aug 30, 2023
@AronBuzogany AronBuzogany deleted the tests branch August 30, 2023 11:50
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.

Add tests from v1

3 participants