ra-data-postgrest
ra-data-postgrest copied to clipboard
raphiniert-com
Who's in for adding unit tests?
This repo is about querying data and is probably used in many productional apps. When adding PR's with new feature, like I've been doing today, we really need unit tests so that we can confidently merge such PR's.
Hey Christian,
I totally am and currently, there is ongoing work for exactly this. I just started some work in the branch improvements. This definitely will be the next step in this library developing. However, there might be some refactoring involved, in order to make the entire code better testable. As said, that's current work. Are you interested to contribute or is this issue solely to be understood as a request?
Kind regards
Cool! I'm kinda interested to contribute. Kinda, because I haven't much time left to do this. I just saw your improvements branch with jest and tests. That'll also probably mean that my other PR's will conflict, but we'll deal with that later.
I just checked out the improvements branch to try and get the existing tests to pass before adding a few of my own.
I ran npm run test and got a few TypeError: fetch failed ... 'ECONNREFUSED' on port: 3000. Do you have Postgrest running locally when running the test?
Would you like me to contribute be creating PR's against the improvements branch from my fork or can I get access permission to commit to this repo directly (with protected master branch of course)?
Hey, sorry, just read your message. I will look into the code I have locally. There should not be the requirement to run post great, but use the mockup data. Further, I can totally grant you access to the branch! I will do that tomorrow!
Hey @christiaanwesterbeek, I just tested the code and wasn't able to reconstruct the failure you got and double checked if everything is on git. On my machine it falwlessly works.
This is a full log of everything from cloning, installing and running the test (npm run test). I use Node.js v18.
[me] git clone https://github.com/raphiniert-com/ra-data-postgrest
Cloning into 'ra-data-postgrest'...
remote: Enumerating objects: 542, done.
remote: Counting objects: 100% (59/59), done.
remote: Compressing objects: 100% (29/29), done.
remote: Total 542 (delta 38), reused 35 (delta 30), pack-reused 483
Receiving objects: 100% (542/542), 296.44 KiB | 7.80 MiB/s, done.
Resolving deltas: 100% (270/270), done.
[me] cd ./ra-data-postgrest
[ra-data-postgrest] git checkout improvements
branch 'improvements' set up to track 'origin/improvements'.
Switched to a new branch 'improvements'
[ra-data-postgrest] npm install
> @raphiniert/[email protected] prepare
> install-peers
- Installing 1 peerDependencies...
+ Successfully installed 1 peerDependencies via npm.
added 17 packages, removed 15 packages, changed 39 packages, and audited 407 packages in 22s
53 packages are looking for funding
run `npm fund` for details
found 0 vulnerabilities
added 404 packages, and audited 405 packages in 34s
53 packages are looking for funding
run `npm fund` for details
found 0 vulnerabilities
[ra-data-postgrest] npm run test
> @raphiniert/[email protected] test
> jest --coverage
FAIL tests/urlBuilder.test.ts
● Test suite failed to run
Your test suite must contain at least one test.
at onResult (node_modules/@jest/core/build/TestScheduler.js:172:18)
at node_modules/@jest/core/build/TestScheduler.js:300:17
at node_modules/emittery/index.js:311:13
at Array.map (<anonymous>)
at Emittery.emit (node_modules/emittery/index.js:309:23)
(node:802) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/deps/undici/undici:6323
p.reject(Object.assign(new TypeError("fetch failed"), { cause: response.error }));
^
TypeError: fetch failed
at Object.processResponse (node:internal/deps/undici/undici:6323:34)
at node:internal/deps/undici/undici:6648:42
at node:internal/process/task_queues:140:7
at AsyncResource.runInAsyncScope (node:async_hooks:203:9)
at AsyncResource.runMicrotask (node:internal/process/task_queues:137:8)
at processTicksAndRejections (node:internal/process/task_queues:95:5) {
cause: Error: connect ECONNREFUSED ::1:3000
at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1300:16) {
errno: -61,
code: 'ECONNREFUSED',
syscall: 'connect',
address: '::1',
port: 3000
}
}
Node.js v18.8.0
(node:845) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/deps/undici/undici:6323
p.reject(Object.assign(new TypeError("fetch failed"), { cause: response.error }));
^
TypeError: fetch failed
at Object.processResponse (node:internal/deps/undici/undici:6323:34)
at node:internal/deps/undici/undici:6648:42
at node:internal/process/task_queues:140:7
at AsyncResource.runInAsyncScope (node:async_hooks:203:9)
at AsyncResource.runMicrotask (node:internal/process/task_queues:137:8)
at processTicksAndRejections (node:internal/process/task_queues:95:5) {
cause: Error: connect ECONNREFUSED ::1:3000
at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1300:16) {
errno: -61,
code: 'ECONNREFUSED',
syscall: 'connect',
address: '::1',
port: 3000
}
}
Node.js v18.8.0
(node:858) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/deps/undici/undici:6323
p.reject(Object.assign(new TypeError("fetch failed"), { cause: response.error }));
^
TypeError: fetch failed
at Object.processResponse (node:internal/deps/undici/undici:6323:34)
at node:internal/deps/undici/undici:6648:42
at node:internal/process/task_queues:140:7
at AsyncResource.runInAsyncScope (node:async_hooks:203:9)
at AsyncResource.runMicrotask (node:internal/process/task_queues:137:8)
at processTicksAndRejections (node:internal/process/task_queues:95:5) {
cause: Error: connect ECONNREFUSED ::1:3000
at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1300:16) {
errno: -61,
code: 'ECONNREFUSED',
syscall: 'connect',
address: '::1',
port: 3000
}
}
Node.js v18.8.0
(node:867) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/deps/undici/undici:6323
p.reject(Object.assign(new TypeError("fetch failed"), { cause: response.error }));
^
TypeError: fetch failed
at Object.processResponse (node:internal/deps/undici/undici:6323:34)
at node:internal/deps/undici/undici:6648:42
at node:internal/process/task_queues:140:7
at AsyncResource.runInAsyncScope (node:async_hooks:203:9)
at AsyncResource.runMicrotask (node:internal/process/task_queues:137:8)
at processTicksAndRejections (node:internal/process/task_queues:95:5) {
cause: Error: connect ECONNREFUSED ::1:3000
at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1300:16) {
errno: -61,
code: 'ECONNREFUSED',
syscall: 'connect',
address: '::1',
port: 3000
}
}
Node.js v18.8.0
FAIL tests/dataProvider.test.ts
● Test suite failed to run
Jest worker encountered 4 child process exceptions, exceeding retry limit
at ChildProcessWorker.initialize (node_modules/jest-worker/build/workers/ChildProcessWorker.js:170:21)
----------|---------|----------|---------|---------|-------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files | 0 | 0 | 0 | 0 |
----------|---------|----------|---------|---------|-------------------
Test Suites: 2 failed, 2 total
Tests: 0 total
Snapshots: 0 total
Time: 10.225 s
Ran all test suites.
[ra-data-postgrest]
When I remove the test "minimal arguments for dataprovider" and remove the empty file urlBuilder.test.ts, then my test passes.
diff --git a/tests/dataProvider.test.ts b/tests/dataProvider.test.ts
index 0e2dcd5..487e5d1 100644
--- a/tests/dataProvider.test.ts
+++ b/tests/dataProvider.test.ts
@@ -8,15 +8,6 @@ type HTTPClientMock = typeof fetchUtils.fetchJson;
describe("test dataProvider", () => {
- const BASE_URL = "http://localhost:3000";
-
- test("minimal arguments for dataprovider", () => {
- const t = () => {
- dataProvider(BASE_URL)
- .getOne("Patient", { id: SINGLE_TODO.id })
- .then((response) => response != undefined);
- };
- expect(t).not.toThrow(TypeError);
- });
-
describe("getList", () => {
test("test page offset, sort and return value", () => {
const httpClient = createHTTPClientMock(
diff --git a/tests/urlBuilder.test.ts b/tests/urlBuilder.test.ts
deleted file mode 100644
index e69de29..0000000
[ra-data-postgrest] npm run test improvements ✗ ✭ ✱
> @raphiniert/[email protected] test
> jest --coverage
PASS tests/dataProvider.test.ts
test dataProvider
getList
✓ test page offset, sort and return value (1 ms)
getOne
✓ with Entity ID
----------------|---------|----------|---------|---------|-------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------------|---------|----------|---------|---------|-------------------
All files | 100 | 100 | 100 | 100 |
mockup.data.ts | 100 | 100 | 100 | 100 |
----------------|---------|----------|---------|---------|-------------------
Test Suites: 1 passed, 1 total
Tests: 2 passed, 2 total
Snapshots: 0 total
Time: 1.49 s
Ran all test suites.
[ra-data-postgrest]
I'll probably be able to mock requests to 3000. But I'm not sure if we need to, because we shouldn't want to mock postgrest request and responses, because we don't want to unit test postgrest functionality as it has its own unit tests. We should only test if ra-data-postgrest is building urls the way we expect it to.
When could you give me write/commit access to this repo?
There's a lot of extra functionality that I would like to add, but without any breaking changes.
I think we totally should mock http requests, as there is more than just generating the URL. We also have to handle specific http-header information. Could you maybe write me an Email with the contact form on raphiniert.com? I would like to setup a zoom call with you, in order to coordinate the contributions, as there was another guy interested to contribute. We should not rush, but plan good, so that the result will be solid and we do not work the same things multiple times.
Accomplished. Nice! https://github.com/raphiniert-com/ra-data-postgrest/pull/53