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

Testing enviornment for Redis Services #2037

Closed
Manik2708 opened this issue Mar 21, 2024 · 17 comments
Closed

Testing enviornment for Redis Services #2037

Manik2708 opened this issue Mar 21, 2024 · 17 comments
Labels
feature request no-issue-activity No issue activity test Testing application unapproved Unapproved for Pull Request

Comments

@Manik2708
Copy link

Is your feature request related to a problem? Please describe.

Currently there are no tests written for src/services. These services are Cache Services and as project is moving forward, a testing environment for this has to be prepared.

Describe the solution you'd like

The solution according to me can be running a testing Redis server on a different port other than standard 6379. Mocking can bypass the tests making them pseudo, hence e2e (end-to-end) tests should be performed.

Approach to be followed (optional)

  1. Firstly think of a possible way of dependency injection. Currently there is no Dependency Injection. A possible library for this can be tsyringe. If you have a better alternative to this, feel free to discuss.
  2. Once library for DI is decided, make sure it is properly injected in all the resolvers and queries.
  3. Now prepare a script for running testing redis instance if already not running. A package called wait-on can be used for this.
  4. Write a test for any file in services so that it is easy for a reviewer to review this PR.

Additional context

Only thing which I want to say that if you have any other better approach, the please firstly discuss with maintainers. If possible firstly propose an approach in the issue and then start working.

Potential internship candidates
Please read this if you are planning to apply for a Palisadoes Foundation internship PalisadoesFoundation/talawa#359

@github-actions github-actions bot added test Testing application unapproved Unapproved for Pull Request labels Mar 21, 2024
@Manik2708
Copy link
Author

@palisadoes If this is relevant then can we talk on this?

@varshith257
Copy link
Member

varshith257 commented Mar 23, 2024

@Manik2708 Once check @xoldyckk 's message in talawa-admin slack channel. The Redis recently moved away from open-source. maybe a case we have to move to another solution instead of Redis in the near future.

@Manik2708
Copy link
Author

Like when it is going to happen? Because till then I think we will be needy of tests. Without that code might break!

@varshith257
Copy link
Member

Might need guidance from @palisadoes for it

@Manik2708
Copy link
Author

@palisadoes xoldyckk This account is not available on GitHub. Please assist!

@Cioppolo14
Copy link
Contributor

@Manik2708 Check for discussions on redis in Slack, there have been some, maybe it will give you the guidance you need.

@Manik2708
Copy link
Author

@Manik2708 Check for discussions on redis in Slack, there have been some, maybe it will give you the guidance you need.

Actually it is not related to Redis, it is related to test Cache Services. Once current Cache Services are tested it will be easy to migrate to a new Caching Database. It will make it less buggy.

@Manik2708
Copy link
Author

I think we don't need to leave Redis. This PR solves the licence problem: redis/redis#13169

@Manik2708
Copy link
Author

I think we don't need to leave Redis. This PR solves the licence problem: redis/redis#13169

This PR was closed just after this comment. I think we can wait for a while before reaching a migration decision.

@xoldd
Copy link
Contributor

xoldd commented Mar 26, 2024

@Manik2708 Once check @xoldyckk 's message in talawa-admin slack channel. The Redis recently moved away from open-source. maybe a case we have to move to another solution instead of Redis in the near future.

It was just for informing Peter, I didn't say anything about needing to replace it. As far as I'm aware, the license doesn't prohibit usage of redis in the way it is used in talawa-api.

@palisadoes xoldyckk This account is not available on GitHub. Please assist!

Changed my username, thought the new one was more appropriate.

@Manik2708
Copy link
Author

@Manik2708 Once check @xoldyckk 's message in talawa-admin slack channel. The Redis recently moved away from open-source. maybe a case we have to move to another solution instead of Redis in the near future.

It was just for informing Peter, I didn't say anything about needing to replace it. As far as I'm aware, the license doesn't prohibit usage of redis in the way it is used in talawa-api.

@palisadoes xoldyckk This account is not available on GitHub. Please assist!

Changed my username, thought the new one was more appropriate.

Ok, thanks a lot for explanation. I think then we can talk on the approach to test Redis Services.

@Manik2708 Manik2708 mentioned this issue Mar 29, 2024
Copy link

github-actions bot commented Apr 7, 2024

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Apr 7, 2024
@Manik2708
Copy link
Author

@palisadoes Actually from the last week I was searching about DI in node js. I have written test including Redis in one of my personal project https://github.com/Manik2708/Hi_Server/blob/develop/test/Controllers/Confession/Services/send_confession.e2e-spec.ts, but problem is that this project is in Nestjs and it has an OOPs based structure. But unfortunately our Talawa-API is neither in Nest nor have OOPs structure. Also nodejs doesn't have a strong DI library.

I was thinking to test Redis Services in this way:

  1. Auto injecting Redis client in the resolvers. When App will start, it will use redis-server running on main port (say 6379).
  2. When running a test, a testing redis server will be used which would be running on a different port (say 6380).

I achieved this in Nestjs, but currently it is nearly impossible to execute this in Nodejs. So either we have ditch tests for redis services for now or we have to migrate from Nodejs+Graphql to Nestjs+Graphql.

Another advice from my side is to consider this migration important because as far I know, industries don't use Node js due to it's poor structure. As nodejs code increases, it becomes very difficult to maintain and test the code.

@palisadoes
Copy link
Contributor

This is not an area of strength for me. If so the slack channel and / or create a GitHub discussion for this

@github-actions github-actions bot removed the no-issue-activity No issue activity label Apr 15, 2024
@xoldd
Copy link
Contributor

xoldd commented Apr 22, 2024

@Manik2708 you can have dependency injection using the graphql context. It is available to all resolvers as the third parameter of the resolver function.

Copy link

github-actions bot commented May 3, 2024

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label May 3, 2024
Copy link

This issue did not get any activity in the past 180 days and thus has been closed. Please check if the newest release or develop branch has it fixed. Please, create a new issue if the issue is not fixed.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request no-issue-activity No issue activity test Testing application unapproved Unapproved for Pull Request
Projects
Status: Done
Development

No branches or pull requests

5 participants