Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit Tests - Node #736

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mvarendorff
Copy link
Contributor

@mvarendorff mvarendorff commented Oct 31, 2023

What does this PR do?

This PR adds generated unit tests to the Node SDK.

Test Plan

Generate the SDK, then run run yarn install and yarn 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

Copy link
Member

@lohanidamodar lohanidamodar left a comment

Choose a reason for hiding this comment

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

From brief review, its looking good. Would test with generation later.

@mvarendorff mvarendorff force-pushed the feat-680-add-unit-tests-node branch from 17b7006 to e4dbb20 Compare November 19, 2023 15:38
@lohanidamodar lohanidamodar requested a review from loks0n December 17, 2023 00:26
@stnguyen90 stnguyen90 removed their request for review December 28, 2023 00:43
@abnegate
Copy link
Contributor

Are you make these test run automatically with the existing tests?

@mvarendorff
Copy link
Contributor Author

mvarendorff commented Feb 25, 2024

@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.

@gewenyu99
Copy link

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.

@mvarendorff mvarendorff mentioned this pull request Apr 8, 2024
@lohanidamodar
Copy link
Member

@abnegate these tests are to be run on appwrite/sdk-for-node repository instead.

@lohanidamodar lohanidamodar requested a review from loks0n April 15, 2024 03:00
@stnguyen90
Copy link
Contributor

these tests are to be run on appwrite/sdk-for-node repository instead.

@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 appwrite/sdk-for-node repo 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants