RocketChat / EmbeddedChat

An easy to use full-stack component (ReactJS) embedding Rocket.Chat into your webapp
https://www.npmjs.com/package/@embeddedchat/react
107 stars 214 forks source link

test: set up authentication #570

Open JeffreytheCoder opened 2 months ago

JeffreytheCoder commented 2 months ago

What this PR does

Acceptance Criteria fulfillment

Fixes #546

Video/Screenshots

test-auth

JeffreytheCoder commented 2 months ago

I changed Playwright's base url to http://localhost:6006 to run tests against EC in storybook. @sidmohanty11 Do we have storybook running in workflow? We might need to make some changes

Spiral-Memory commented 2 months ago

Hey @JeffreytheCoder , I have some questions in mind. I don't have much understanding in setting up these tests, but I want to learn more about it so that I can also contribute by writing e2e tests, as the central issue is huge. I was going through your changes, and I would like to ask a few questions. It would be great if you could spare some of your time answering them and help me understand.

Spiral-Memory commented 2 months ago

What each of these is doing and why did you keep 2 different ports for this? Earlier both of them were the same. After changing it to localhost:6000 and localhost:5173, I can see that Playwright tests are failing in your PR, and it says that it is unable to connect to that, and the connection is refused maybe because server is started on different port and you are trying to connect to a different port. However, before, it used to connect and have a check on basic things like whether the chat header is present or not, and it obviously happens if it is able to see the UI. I might be asking silly question, I am not exactly how it works properly, could you please help me understand it ?

image Screenshot 2024-04-14 104747 image
Spiral-Memory commented 2 months ago

I would like to know why you removed the host. Without a host, how will it connect to the Rocket.chat server? I believe chat.avitechlab.com was a hosted Rocket.Chat server on which api call could be made and tested on. Could you please help me understand this change?

image
JeffreytheCoder commented 2 months ago

Hey @Spiral-Memory I was confused with whether the tests should run in development environment or in a dedicated testing environment in CI. I changed host to storybook and added environment variables so that each EC dev can run tests using his/her own server. However, if we only want to run tests during CI, I'll change it back to using chat.avitechlab.com.

JeffreytheCoder commented 2 months ago

Before this PR, EC was set to anonymous mode for CI tests. I was thinking we can just discard the authentication setup, but then I saw EC currently only allows anonymous read, not anonymous write. If this is a feature, not a bug, we then need a test account of the hosted server to sign in and test the messaging features.

Spiral-Memory commented 2 months ago

Hey @Spiral-Memory, I was confused about whether the tests should run in the development environment or in a dedicated testing environment in CI. I changed the host to Storybook and added environment variables so that each EC dev can run tests using their own server. However, if we only want to run tests during CI, I'll change it back to using chat.avitechlab.com.

In my opinion, tests should run on CI to achieve an automated workflow. Additionally, the environment variables for login are fine; the repository owner can add those environment variables to enable automated login on chat.avitechlab.com.

Also, we have two modes in RC. One is anonymous mode, which allows us to read messages if the workspace owner has enabled it. So, there's no need to remove that. Let's check for the anonymous mode first. Once the test passes, we can proceed to login and test the other features which require write access accordingly.

Also login is necessary to test other APIs that require auth, so we can't avoid login at all. anonymous mode is given by RC so that user can at least read messages without creating a account, and this is specific to workspace owner, they have the permission to enable / disable it and In EC, we need to test all scenarios, so testing both could be beneficial.

Regarding the test user, I think Abhinav has hosted that server so he or siddarth will add the credentials himself for test with admin access.

Spiral-Memory commented 2 months ago

What each of these is doing and why did you keep 2 different ports for this? Earlier both of them were the same. After changing it to localhost:6000 and localhost:5173, I can see that Playwright tests are failing in your PR, and it says that it is unable to connect to that, and the connection is refused maybe because server is started on different port and you are trying to connect to a different port. However, before, it used to connect and have a check on basic things like whether the chat header is present or not, and it obviously happens if it is able to see the UI. I might be asking silly question, I am not exactly how it works properly, could you please help me understand it ?

image Screenshot 2024-04-14 104747 image

Also, once we refer to this, I think one is to start the EC, and the other part will go to that URL and test. So, keep it consistent, and it should be the same to successfully test things. Again, I'm not very sure how testing works, but this is what makes sense to me. Let's give it a try.

Also, For testing in your local system, use your Rocket chat server URL instead of chat.avitechlab.com and while raising PR, use this, once the maintainer add their credentials it will start working.

JeffreytheCoder commented 2 months ago

Hey @abhinavkrin @sidmohanty11 do you guys have the admin credentials to the test server chat.avitechlab.com?

Spiral-Memory commented 2 months ago

Hey @abhinavkrin @sidmohanty11 do you guys have the credentials to the test server chat.avitechlab.com?

Obviously they will have ig 😂, for testing, btw you can create your own account on that (avitech) Rocket.Chat server for now. I just created one for me. (but not with admin access obviously)

JeffreytheCoder commented 2 months ago

Hey @abhinavkrin @sidmohanty11 do you guys have the credentials to the test server chat.avitechlab.com?

Obviously they will have ig 😂, for testing, btw you can create your own account on that (avitech) Rocket.Chat server for now. I just created one for me. (but not with admin access obviously)

Ohh that's great. I thought we need to share a test account so that's why I asked haha

Spiral-Memory commented 2 months ago

Hey @abhinavkrin @sidmohanty11 do you guys have the credentials to the test server chat.avitechlab.com?

Obviously they will have ig 😂, for testing, btw you can create your own account on that (avitech) Rocket.Chat server for now. I just created one for me. (but not with admin access obviously)

Ohh that's great. I thought we need to share a test account so that's why I asked haha

It depends on Abhinav.

Also, we might need to test some features with admin access, so I'm a little unsure if Abhinav will provide a test user with admin access. Haha... Maybe we can test non-admin related features with our own credentials on that server or using a common shared test user (without admin access / with admin access, depending on Abhinav's choice). But I think he is quite busy these days, so for now let's use our credentials to conduct testing on the local system (either with our local server (if admin access is required) or with the Avitechlab server).

JeffreytheCoder commented 2 months ago

Anyway we need a shared account for CI to use. I created a tester account and we can change to an admin account later

Screenshot 2024-04-13 at 11 41 57 PM
Spiral-Memory commented 2 months ago

Yup ! Great thanks for sharing, but i think for CI, abhinav or siddarth will add an account with admin access in GitHub workflow credentials.

Spiral-Memory commented 2 months ago

Anyway, let's start working on it.. once the login is complete and once you make the required changes to this PR. Let me know, we will test together !!

JeffreytheCoder commented 2 months ago

Added a new commit and the tests now work in CI :)

Spiral-Memory commented 2 months ago

@sidmohanty11 Could you once add credentials in GitHub workflow directly so that it can be fetched from. env rather than hardcoding a test user.

Spiral-Memory commented 2 months ago

Added a new commit and the tests now work in CI :)

Great but you have removed some tests, like seeing the messages and stuff from the example.spec.ts, once revert those as well.

Also, after logging in, we need to check things like whether the input box is now enabled, whether the "Login successful" toast message is rendered, etc., to verify that the login has been successful, in my opinion. Please mark it as a draft for now, and once it's done, make it ready for review to avoid confusion.

JeffreytheCoder commented 2 months ago

Added a new commit and the tests now work in CI :)

Great but you have removed some tests, like seeing the messages and stuff from the example.spec.ts, once revert those as well.

Also, after logging in, we need to check things like whether the input box is now enabled, whether the "Login successful" toast message is rendered, etc., to verify that the login has been successful, in my opinion. Please mark it as a draft for now, and once it's done, make it ready for review to avoid confusion.

Let's wait until the meeting to verify these changes