Conversation
mvarendorff
force-pushed
the
feat-680-add-unit-tests-node
branch
from
3629a9c to
17b7006
Compare
lohanidamodar
left a comment
lohanidamodar
left a comment
There was a problem hiding this comment.
From brief review, its looking good. Would test with generation later.
mvarendorff
force-pushed
the
feat-680-add-unit-tests-node
branch
from
17b7006 to
e4dbb20
Compare
|
Are you make these test run automatically with the existing tests? |
|
@abnegate No, and I wasn't aware that was part of the task, sorry! Same answer goes for the other PRs from me you commented on. Since there currently seems to be a fairly central point of testing which is broken up by these PRs, it might make sense to overhaul it in one go at some point? Two of the Unit Tests PRs (PHP and Deno) are also affected. Edit: I just double checked to see why that went under my radar and it seems that the PR referenced in the original issue also did not include any automatic testing, so in addition to PHP and Deno, Flutter + Dart are also affected. |
|
Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship. Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag. |
|
@abnegate these tests are to be run on |
@lohanidamodar, for Flutter and Dart, we run them in this repo. I think it makes sense to run them in this repo to catch any problems early before pushing to the |
|
Quick triage check: is this PR still active? It’s been inactive for a long time and there has been newer testing work since then. If you still plan to continue, please share status/rebase plan; otherwise we can close it for now. |
|
@ChiragAgg5k commenting here for all 4 of my PRs: I will check how trivial a rebase would be in the next few days but after 2 years of changes, I have a hunch that it's going to take longer than I currently have capacity for. Depending on the results I will either fix the PRs or close them accordingly. |
mvarendorff
force-pushed
the
feat-680-add-unit-tests-node
branch
from
e4dbb20 to
58da2df
Compare
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request adds comprehensive test infrastructure to the Node.js SDK generator. The Node class now extends JS instead of Web, and multiple test template files are introduced for core modules (id, permission, role, query) and service wrappers. Jest is added as a devDependency to support test execution. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/node/package.json.twig (1)
9-11:⚠️ Potential issue | 🟡 MinorMissing
testscript to run Jest tests.The
scriptssection only has"build": "tsup"but lacks a"test"script. Without this, users cannot runyarn testornpm testto execute the generated tests.Proposed fix
"scripts": { - "build": "tsup" + "build": "tsup", + "test": "jest" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/node/package.json.twig` around lines 9 - 11, Add a "test" script to the package.json templates so generated projects can run Jest; update the "scripts" object (which currently contains "build": "tsup") to include a "test" entry (e.g., "test": "jest") so users can run `npm test`/`yarn test` to execute tests using Jest.
🧹 Nitpick comments (4)
templates/node/test/query.test.js.twig (1)
155-155: Missing semicolon after closingdescribeblock.Proposed fix
-}) +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/node/test/query.test.js.twig` at line 155, Add the missing semicolon after the closing describe block; locate the closing "})" that ends the describe(...) in query.test.js.twig and append a semicolon so the statement is properly terminated (i.e., change the final "})" of the describe block to "});").templates/node/test/services/service.test.js.twig (1)
37-38: Missing semicolon after closingdescribeblock.Proposed fix
-}) +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/node/test/services/service.test.js.twig` around lines 37 - 38, Add a missing semicolon after the closing describe block so the test file's IIFE/statement is properly terminated: locate the closing "})" that ends the describe block in templates' generated test (the describe(...) { ... } block) and append a semicolon immediately after it to produce "});".templates/node/test/role.test.js.twig (1)
10-14: Missing semicolons in test statements.Lines 10-13 are missing semicolons after the closing parentheses, and line 14 is missing a semicolon after the
describeblock. This is inconsistent with the rest of the file.Proposed fix
- test('team without role', () => expect(Role.team('custom')).toEqual('team:custom')) - test('team with role', () => expect(Role.team('custom', 'owner')).toEqual('team:custom/owner')) - test('member', () => expect(Role.member('custom')).toEqual('member:custom')) - test('label', () => expect(Role.label('admin')).toEqual('label:admin')) -}) + test('team without role', () => expect(Role.team('custom')).toEqual('team:custom')); + test('team with role', () => expect(Role.team('custom', 'owner')).toEqual('team:custom/owner')); + test('member', () => expect(Role.member('custom')).toEqual('member:custom')); + test('label', () => expect(Role.label('admin')).toEqual('label:admin')); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/node/test/role.test.js.twig` around lines 10 - 14, Add missing semicolons to the test statements and the closing describe to match code style: append a semicolon after each test invocation for Role.team, Role.member, and Role.label (the test(...) calls) and add a semicolon after the final describe block closure so the lines containing Role.team, Role.member, Role.label, and the describe(...) closing bracket all end with semicolons.templates/node/test/permission.test.js.twig (1)
10-10: Missing semicolon after closing parenthesis.For consistency with the other test files and JavaScript conventions, add a semicolon after the closing
}).Proposed fix
-}) +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/node/test/permission.test.js.twig` at line 10, Add a missing semicolon after the closing test-block token "})" in the permission test (the end of the test suite in permission.test.js.twig); locate the final "})" that closes the describe/it block and append a semicolon to make it "});" for consistency with other tests and JS conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/SDK/Language/Node.php`:
- Around line 193-197: The destination path for the service test entry contains
a leading slash and is inconsistent with other test paths; update the array
entry where 'destination' => '/test/services/{{service.name |
caseDash}}.test.js' (in the Node language spec) to remove the leading slash so
it matches the other entries (i.e., use 'test/services/{{service.name |
caseDash}}.test.js'); keep the 'template' =>
'node/test/services/service.test.js.twig' and other keys unchanged.
- Line 5: Node.php currently extends JS but depends on Web's Twig filters (e.g.,
getPropertyType, getReturn, getGenerics, getOverloadCondition) so generation
fails; fix by either changing the class declaration back to extend Web (class
Node extends Web) or implement Node::getFilters() that returns/merges the same
filter definitions provided by Web::getFilters() including those named filters;
additionally, normalize template destination paths in Node (remove inconsistent
leading '/' from strings like '/test/services/...' and
'/node/tsconfig.json.twig' so they match other entries such as
'test/permission.test.js') to ensure consistent path handling.
In `@templates/node/test/query.test.js.twig`:
- Around line 115-118: In the 'with strings' unit test for Query.between, the
expected string is missing a comma between the second and third arguments;
update the expectation in the test named 'with strings' that calls
Query.between('attr', "a", "z") so it asserts toEqual('between("attr", "a",
"z")') (i.e., add the comma between "a" and "z" to match the numeric variants
and the Query.between format).
---
Outside diff comments:
In `@templates/node/package.json.twig`:
- Around line 9-11: Add a "test" script to the package.json templates so
generated projects can run Jest; update the "scripts" object (which currently
contains "build": "tsup") to include a "test" entry (e.g., "test": "jest") so
users can run `npm test`/`yarn test` to execute tests using Jest.
---
Nitpick comments:
In `@templates/node/test/permission.test.js.twig`:
- Line 10: Add a missing semicolon after the closing test-block token "})" in
the permission test (the end of the test suite in permission.test.js.twig);
locate the final "})" that closes the describe/it block and append a semicolon
to make it "});" for consistency with other tests and JS conventions.
In `@templates/node/test/query.test.js.twig`:
- Line 155: Add the missing semicolon after the closing describe block; locate
the closing "})" that ends the describe(...) in query.test.js.twig and append a
semicolon so the statement is properly terminated (i.e., change the final "})"
of the describe block to "});").
In `@templates/node/test/role.test.js.twig`:
- Around line 10-14: Add missing semicolons to the test statements and the
closing describe to match code style: append a semicolon after each test
invocation for Role.team, Role.member, and Role.label (the test(...) calls) and
add a semicolon after the final describe block closure so the lines containing
Role.team, Role.member, Role.label, and the describe(...) closing bracket all
end with semicolons.
In `@templates/node/test/services/service.test.js.twig`:
- Around line 37-38: Add a missing semicolon after the closing describe block so
the test file's IIFE/statement is properly terminated: locate the closing "})"
that ends the describe block in templates' generated test (the describe(...) {
... } block) and append a semicolon immediately after it to produce "});".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 661c342a-93d8-41ef-a5c2-664227a51e77
📒 Files selected for processing (7)
src/SDK/Language/Node.phptemplates/node/package.json.twigtemplates/node/test/id.test.js.twigtemplates/node/test/permission.test.js.twigtemplates/node/test/query.test.js.twigtemplates/node/test/role.test.js.twigtemplates/node/test/services/service.test.js.twig
src/SDK/Language/Node.php
Outdated
| namespace Appwrite\SDK\Language; | ||
|
|
||
| class Node extends Web | ||
| class Node extends JS |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check which classes extend Node and might be affected
ast-grep --pattern $'class $_ extends Node {
$$$
}'
# Check if Web.php has additional filters that Node might need
rg -n "getFilters" src/SDK/Language/Web.php -A 30Repository: appwrite/sdk-generator
Length of output: 50378
🏁 Script executed:
#!/bin/bash
# Find ReactNative class definition
rg -n "class ReactNative" src/SDK/Language/ -A 2
# Find Web.php getFilters method
rg -n "getFilters" src/SDK/Language/Web.php -B 2 -A 20
# Find JS.php getFilters method
rg -n "getFilters" src/SDK/Language/JS.php -B 2 -A 20
# Check Node.php current state - look at getFiles() for the leading slash issue
rg -n "destination.*=>.*'/test" src/SDK/Language/Node.phpRepository: appwrite/sdk-generator
Length of output: 2742
🏁 Script executed:
#!/bin/bash
# Check Node.php line 5 - the actual base class
sed -n '1,10p' src/SDK/Language/Node.php
# Get full JS.php getFilters() for comparison
sed -n '225,290p' src/SDK/Language/JS.php
# Find all instances of leading slashes in Node.php getFiles()
rg -n "destination.*=>.*'/" src/SDK/Language/Node.phpRepository: appwrite/sdk-generator
Length of output: 2767
🏁 Script executed:
#!/bin/bash
# Check if CLI or Node templates use those Web-specific filters
rg -n "getPropertyType|getReadOnlyProperties|getSubSchema|getGenerics|getReturn|getOverloadCondition" templates/cli/ templates/node/ 2>/dev/null | head -20
# Check if Node.php extends Web in the ORIGINAL change - look at the git diff or just verify current state
# Let's also verify what filters Node.php itself defines
rg -n "public function getFilters" src/SDK/Language/Node.php -A 50Repository: appwrite/sdk-generator
Length of output: 5041
🏁 Script executed:
#!/bin/bash
# Check if Node.php defines getFilters() to add the missing filters
sed -n '1,500p' src/SDK/Language/Node.php | rg -n "getFilters" -A 40Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Get the full Node.php file to understand its structure
cat src/SDK/Language/Node.php | head -100
# Also check Web.php to see what it extends and what filters it adds
head -100 src/SDK/Language/Web.phpRepository: appwrite/sdk-generator
Length of output: 5863
🏁 Script executed:
#!/bin/bash
# Get the complete Node.php file
cat src/SDK/Language/Node.php
# Also get Web.php's complete getFilters() to see all filters
rg -n "public function getFilters" src/SDK/Language/Web.php -A 100Repository: appwrite/sdk-generator
Length of output: 12341
Node.php cannot extend JS without also providing Web's Twig filters.
Node templates require filters (getPropertyType, getReturn, getGenerics, getOverloadCondition, etc.) that are defined in Web.php's getFilters() method. Since JS.php only defines basic filters and Node doesn't override getFilters(), extending JS directly will break Node SDK generation with missing filter errors.
Additionally, line 195 has an inconsistent leading slash in the destination path ('/test/services/...') compared to other entries like line 189 ('test/permission.test.js') and line 229 (template path /node/tsconfig.json.twig also inconsistent with other templates).
Either restore Node to extend Web, or add the missing filter definitions to Node's getFilters() method. Also standardize the leading slashes in destination paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/SDK/Language/Node.php` at line 5, Node.php currently extends JS but
depends on Web's Twig filters (e.g., getPropertyType, getReturn, getGenerics,
getOverloadCondition) so generation fails; fix by either changing the class
declaration back to extend Web (class Node extends Web) or implement
Node::getFilters() that returns/merges the same filter definitions provided by
Web::getFilters() including those named filters; additionally, normalize
template destination paths in Node (remove inconsistent leading '/' from strings
like '/test/services/...' and '/node/tsconfig.json.twig' so they match other
entries such as 'test/permission.test.js') to ensure consistent path handling.
| [ | ||
| 'scope' => 'service', | ||
| 'destination' => '/test/services/{{service.name | caseDash}}.test.js', | ||
| 'template' => 'node/test/services/service.test.js.twig', | ||
| ], |
There was a problem hiding this comment.
Leading slash in destination path is inconsistent.
Line 195 has '/test/services/...' with a leading slash, while other test paths (lines 150, 160, 170, 180) use 'test/...' without a leading slash. This inconsistency may cause the service tests to be generated at an unexpected location.
Proposed fix
[
'scope' => 'service',
- 'destination' => '/test/services/{{service.name | caseDash}}.test.js',
+ 'destination' => 'test/services/{{service.name | caseDash}}.test.js',
'template' => 'node/test/services/service.test.js.twig',
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [ | |
| 'scope' => 'service', | |
| 'destination' => '/test/services/{{service.name | caseDash}}.test.js', | |
| 'template' => 'node/test/services/service.test.js.twig', | |
| ], | |
| [ | |
| 'scope' => 'service', | |
| 'destination' => 'test/services/{{service.name | caseDash}}.test.js', | |
| 'template' => 'node/test/services/service.test.js.twig', | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/SDK/Language/Node.php` around lines 193 - 197, The destination path for
the service test entry contains a leading slash and is inconsistent with other
test paths; update the array entry where 'destination' =>
'/test/services/{{service.name | caseDash}}.test.js' (in the Node language spec)
to remove the leading slash so it matches the other entries (i.e., use
'test/services/{{service.name | caseDash}}.test.js'); keep the 'template' =>
'node/test/services/service.test.js.twig' and other keys unchanged.
| test('with strings', () => | ||
| expect(Query.between('attr', "a", "z")) | ||
| .toEqual('between("attr", "a" "z")') | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Query.between implementation for string handling
fd -e ts -e js "query" templates/web/src templates/node/src --exec cat {}Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
find . -type f -name "*.ts" -o -name "*.js" -o -name "*.twig" | xargs grep -l "between.*attr" | head -20Repository: appwrite/sdk-generator
Length of output: 713
🏁 Script executed:
fd -type f "query" --extension ts --extension js --extension twigRepository: appwrite/sdk-generator
Length of output: 236
🏁 Script executed:
rg "between\(" --type ts --type js -A 3 -B 3 | head -50Repository: appwrite/sdk-generator
Length of output: 2126
🏁 Script executed:
cat templates/web/src/query.ts.twigRepository: appwrite/sdk-generator
Length of output: 17362
🏁 Script executed:
cat templates/node/src/query.ts.twigRepository: appwrite/sdk-generator
Length of output: 130
🏁 Script executed:
cat templates/node/test/query.test.js.twig | head -130Repository: appwrite/sdk-generator
Length of output: 3606
🏁 Script executed:
find . -name "query.js" -o -name "query.ts" | grep -E "(lib|dist)" | head -10Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
rg "between.*function\|between.*=>" --type ts --type js -A 5 -B 2 | head -80Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
find templates -name "*query*" -type f | grep nodeRepository: appwrite/sdk-generator
Length of output: 104
🏁 Script executed:
cat templates/node/lib/query.js.twigRepository: appwrite/sdk-generator
Length of output: 130
🏁 Script executed:
find templates -type f -name "*query*"Repository: appwrite/sdk-generator
Length of output: 434
🏁 Script executed:
cat templates/web/src/query.ts.twig | grep -A 30 "between"Repository: appwrite/sdk-generator
Length of output: 4242
🏁 Script executed:
cat templates/deno/src/query.ts.twig | head -60Repository: appwrite/sdk-generator
Length of output: 1919
🏁 Script executed:
rg "node.*query\|query.*node" --type json --type yaml --type toml | head -20Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
find . -name "*.json" -o -name "*.yaml" -o -name "*.yml" | xargs grep -l "node\|query" 2>/dev/null | head -10Repository: appwrite/sdk-generator
Length of output: 404
🏁 Script executed:
cat package.json 2>/dev/null | head -50Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
find . -name "README*" -o -name "*.md" | xargs grep -l "query\|generate" 2>/dev/null | head -5Repository: appwrite/sdk-generator
Length of output: 182
🏁 Script executed:
cat README.md | head -100Repository: appwrite/sdk-generator
Length of output: 4009
🏁 Script executed:
ls -la | grep -E "config|generate|spec"Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
find . -name "*.php" -o -name "*.py" -o -name "*.js" | xargs grep -l "between.*format\|query.*format" 2>/dev/null | head -10Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
rg "toString|format" templates/node/test/query.test.js.twig -A 2 -B 2Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
rg "between\(" tests/languages/ -A 1 -B 1Repository: appwrite/sdk-generator
Length of output: 6594
🏁 Script executed:
cat tests/languages/node/test.js | grep -A 5 -B 5 "between"Repository: appwrite/sdk-generator
Length of output: 693
🏁 Script executed:
find . -path "*node*" -name "*.js" -o -path "*node*" -name "*.ts" | xargs grep -l "class Query\|function Query" 2>/dev/null | head -10Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
rg "toString\(\)" templates/ --type ts --type js -B 10 | grep -A 10 "between"Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
cat tests/languages/node/test.js | head -50Repository: appwrite/sdk-generator
Length of output: 1235
🏁 Script executed:
find . -name "dist" -o -name "build" -o -name "examples" | head -10Repository: appwrite/sdk-generator
Length of output: 116
🏁 Script executed:
find . -path "*examples*" -name "*query*" -type f | head -10Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
cat templates/node/test/query.test.js.twig | sed -n '100,130p'Repository: appwrite/sdk-generator
Length of output: 920
🏁 Script executed:
find . -name "*.php" | xargs grep -l "class Query\|function between" 2>/dev/null | head -5Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
rg "between.*attr.*a.*z" --type js --type ts --type phpRepository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
cat src/SDK/Language/Node.php 2>/dev/null | head -100Repository: appwrite/sdk-generator
Length of output: 2867
🏁 Script executed:
cat src/SDK/Language/JS.php | head -200Repository: appwrite/sdk-generator
Length of output: 5309
🏁 Script executed:
rg "between.*format\|query.*method\|toString" src/SDK/Language/JS.php -A 5 -B 5Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
cat templates/node/test/query.test.js.twig | grep -A 200 "describe.*between" | head -50Repository: appwrite/sdk-generator
Length of output: 1367
🏁 Script executed:
rg "between\|Query\." templates/node/test/query.test.js.twig | head -30Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
cat templates/node/test/query.test.js.twigRepository: appwrite/sdk-generator
Length of output: 4172
🏁 Script executed:
find . -name "*.twig" -path "*query*" | xargs grep -l "between\|method" | head -10Repository: appwrite/sdk-generator
Length of output: 454
🏁 Script executed:
cat templates/node/src/query.js.twig 2>/dev/null || cat templates/node/src/query.ts.twig 2>/dev/null || find templates/node -name "*.twig" -type fRepository: appwrite/sdk-generator
Length of output: 783
🏁 Script executed:
rg "class Query|between" templates/ -A 10 -B 2 | grep -A 10 "between" | head -50Repository: appwrite/sdk-generator
Length of output: 3237
Fix missing comma in between test expectation for strings.
Line 117 expects 'between("attr", "a" "z")' but should be 'between("attr", "a", "z")' to match the format used for numeric variants on lines 109 and 113.
Current code (lines 115-118)
test('with strings', () =>
expect(Query.between('attr', "a", "z"))
.toEqual('between("attr", "a" "z")')
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates/node/test/query.test.js.twig` around lines 115 - 118, In the 'with
strings' unit test for Query.between, the expected string is missing a comma
between the second and third arguments; update the expectation in the test named
'with strings' that calls Query.between('attr', "a", "z") so it asserts
toEqual('between("attr", "a", "z")') (i.e., add the comma between "a" and "z" to
match the numeric variants and the Query.between format).
|
@ChiragAgg5k I think it should be doable to get these polished up again - frankly, I am lacking the resources to bring everything to the most up-to-date state (e.g. include potentially new / more in-depth test-cases) but I am down to put some best-effort into these. This PR I have mostly gotten ready, getting all tests fixed, I am going to throw together a quick operator.test.ts in the next couple days and then look into the other SDKs (Android/Kotlin and Python are both still on the list - .NET is superseeded by a more recent PR, I closed mine already). Since that had been a comment in the past: I am not sure entirely how to set up automatic testing. In case that's required in this PR, I'd appreciate some guidance for that! And another thing before I sink too much time into these: Do you think the PRs would be merged in the weeks after it's done? My schedule is going to be very tight later this year to the point where I cannot support the PRs anymore and I don't have a rough idea when I'd be free again and it would be unlucky for the time to go wasted. |
|
@mvarendorff thanks for the quick response! really happy to see you picked them up. be assured the work wont go to waste, i will try my best to do quick reviews. you can also reach out to me on discord -> |
|
@mvarendorff approved workflows, seems node build validation is failing, lets try get it passed. also mark the PR ready for review when ur done 👌 |
7 participants
What does this PR do?
This PR adds generated unit tests to the Node SDK.
Test Plan
Generate the SDK, then run run
yarn installandyarn jest(or the equivalents for your favourite package manager) in the directory of the generated SDK.Related PRs and Issues
#680
Have you read the Contributing Guidelines on issues?
Yup
Discord username for swag as requested by Tessa: yestheory
Summary by CodeRabbit