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

geisterfurz007
Copy link
Contributor

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

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

@geisterfurz007
Copy link
Contributor Author

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

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

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

None yet

6 participants