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

Update socket tests #185

Merged
merged 4 commits into from
Feb 26, 2024
Merged

Update socket tests #185

merged 4 commits into from
Feb 26, 2024

Conversation

aaditkamat
Copy link
Collaborator

@aaditkamat aaditkamat commented Feb 25, 2024

Related to #177.

The issue was likely because there was no ServerSocket object created to accept requests from the client and give an appropriate response, which is why the tests waited for the requests to complete and timed out when they did not. I also had to make sure that the done callback taken in as a parameter within the test functions was called only once all the testing logic had been implemented.

I mainly referred to these two documents to rewrite the tests:

  1. Socket.io testing
  2. How to test Socket.io with Jest on backend (Node.js)?

Also, to allow multiple client connections, it is good to enable this option within the SocketClient object, which I have done.

This PR should be merged before #180, because those changes have not been accounted for

@h1divp
Copy link
Collaborator

h1divp commented Feb 26, 2024

Delicious PR
My only concern is with the very last test case, since it seems that its being passed just because done() is called at the very end, instead of verifying if the specific message the test client sends would be received. I'm just going to add a comment since I believe Mohammad is working on that issue. I'm fine with the fake passing for now.

@h1divp h1divp self-requested a review February 26, 2024 02:47
@h1divp
Copy link
Collaborator

h1divp commented Feb 26, 2024

Besides that, the functionality works great. Thank you for your work and insight!

@h1divp h1divp merged commit 3f662e7 into ufosc:main Feb 26, 2024
1 check passed
AlexanderWangY pushed a commit that referenced this pull request Feb 26, 2024
* Update socket tests (#185)

* Update socket tests

* Remove unused type

* Update comment in socketio.test.ts

---------

Co-authored-by: Phoenix <[email protected]>

* Create subdirectory before creating service account JSON

---------

Co-authored-by: Phoenix <[email protected]>
h1divp added a commit that referenced this pull request Feb 29, 2024
* Set up Firebase-Admin in adminInit.ts (#155)

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* added passport

* started auth + admin sdk

* removed passport

* added private key to gitignore

* Delete server/private_keys/private.json

---------

Co-authored-by: AlexanderWangY <[email protected]>

* Refactored most of the actions (#156)

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* added passport

* started auth + admin sdk

* removed passport

* added private key to gitignore

* Delete server/private_keys/private.json

* initializing firestore in admin

* finished most of action refactoring

* Made error case for email in use display correctly in front end (#158)

* changed getConnectedUsers to admin

* migrated to firebase-admin

---------

Co-authored-by: AlexanderWangY <[email protected]>
Co-authored-by: Mohammed Ali <[email protected]>

* Added JWT on top of Firebase-Admin (#178)

* Made error case for email in use display correctly in front end

* Made error case for email in use display correctly in front end (#158)

* Cleaned up useEffect function #159

* Added testing pipeline (#175)

* Create main.yml

* Update main.yml

* Update _layout.tsx

* Update main.yml

* Update socketio.test.ts

* Update socketio.test.ts

* Update _layout.tsx

* User Context and User Type created and wrapped chat screen (#131)

* added user context

* Added userID and displayName

* moved user and display name generation into UserProvider

* Improved UserContext implementation

Amongst these changes, the user type on the frontend has been edited in order
to prevent hidden functions that would pull information into the UserContext
from other contexts. The message type was also modified to keep a new author
section with a displayName attribute. An issue should be created for a socket
API endpoint for grabbing the displayName, and one to set it in a connectedUser
document in the databse.

* updated package lock

---------

Co-authored-by: h1divp <[email protected]>

* Set up Firebase-Admin in adminInit.ts (#155)

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* added passport

* started auth + admin sdk

* removed passport

* added private key to gitignore

* Delete server/private_keys/private.json

---------

Co-authored-by: AlexanderWangY <[email protected]>

* Refactored most of the actions (#156)

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* fixes pt2

* JWT WIP

* new branch + function fix

* small bug fixes

* added passport

* started auth + admin sdk

* removed passport

* added private key to gitignore

* Delete server/private_keys/private.json

* initializing firestore in admin

* finished most of action refactoring

* Made error case for email in use display correctly in front end (#158)

* changed getConnectedUsers to admin

* migrated to firebase-admin

---------

Co-authored-by: AlexanderWangY <[email protected]>
Co-authored-by: Mohammed Ali <[email protected]>

* added middleware

---------

Co-authored-by: Phantom0110 <[email protected]>
Co-authored-by: Mohammed Ali <[email protected]>
Co-authored-by: AaronGibson2 <[email protected]>
Co-authored-by: h1divp <[email protected]>
Co-authored-by: AlexanderWangY <[email protected]>

* Delete server/src/private_key/private.json

* Update main.yml

* Updated pipeline to reflect firebase-admin

* Update main.yml

* Update main.yml

* Create subdirectory before creating service account JSON (#187)

* Update socket tests (#185)

* Update socket tests

* Remove unused type

* Update comment in socketio.test.ts

---------

Co-authored-by: Phoenix <[email protected]>

* Create subdirectory before creating service account JSON

---------

Co-authored-by: Phoenix <[email protected]>

* made changes to collection name + socket test load testing

* Added socketIo.connect()

* Remade collection name changes

* updated location of firebase secrets, added comment about it in config_example.md

* renamed file to firebaseInit from adminInit

* Updated comment

---------

Co-authored-by: AlexanderWangY <[email protected]>
Co-authored-by: Mohammed Ali <[email protected]>
Co-authored-by: Phantom0110 <[email protected]>
Co-authored-by: AaronGibson2 <[email protected]>
Co-authored-by: h1divp <[email protected]>
Co-authored-by: Aadit Kamat <[email protected]>
@Phantom0110 Phantom0110 mentioned this pull request Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants