Add unit-testing for search, functions and fetching #18

New Issue

Closed

opened 2024-03-30 14:06:10 +00:00 by timharek · 4 comments

timharek commented

2024-03-30 14:06:10 +00:00

(Migrated from github.com)

This will help when deploying new versions as well as help others contribute.

timharek commented

2024-04-04 20:33:26 +00:00

(Migrated from github.com)

There seems to be leaks coming from https://github.com/olaven/kall, which makes it difficult to make proper tests.

Should I fix the issue upstream in olaven/kall or should I replace kall with ordinary fetch within the project?

cc @olaven

There seems to be leaks coming from https://github.com/olaven/kall, which makes it difficult to make proper tests. Should I fix the issue upstream in olaven/kall or should I replace `kall` with ordinary `fetch` within the project? <img width="1758" alt="image" src="https://github.com/olaven/NRSS/assets/5453010/21b6c710-ab41-432f-a3a2-0c9d77f0f6bf"> cc @olaven

olaven commented

2024-04-05 08:02:19 +00:00

(Migrated from github.com)

@timharek that's no good! thank you for bringing ths up. If you want to fix the error upstream, that's the best solution IMO.

I think the issue is that the response is only read when the application type is json, based on this check, which might not be complete enough. So I think the fix would be to 1) consume the response body regardless of content type and 2) (perhaps) update the content type check.

@timharek that's no good! thank you for bringing ths up. If you want to fix the error upstream, that's the best solution IMO. I think the issue is that the response [is only read](https://github.com/olaven/kall/blob/master/source/support.ts#L32-L34) when the application type is json, based on [this check](https://github.com/olaven/kall/blob/7117f02496a5f315fa8b4f51e9fa3bb8bf28a1c0/source/support.ts#L1-L3), which might not be complete enough. So I think the fix would be to 1) consume the response body regardless of content type and 2) (perhaps) update the content type check.

timharek commented

2024-04-05 14:00:23 +00:00

(Migrated from github.com)

I'll look into it upstream 😊

I see that some of the tests in kall also fail with the same error, so I'll know when it's fixed. I'll ping you when I'm done.

I'll look into it upstream 😊 I see that some of the tests in kall also fail with the same error, so I'll know when it's fixed. I'll ping you when I'm done.

timharek commented

2024-04-05 14:03:05 +00:00

(Migrated from github.com)

Also found your old issue regarding the same issue https://github.com/denoland/deno/issues/4735