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

[Feature Request] Add Unit Tests #125

Closed
Helloyunho opened this issue Apr 14, 2021 · 12 comments
Closed

[Feature Request] Add Unit Tests #125

Helloyunho opened this issue Apr 14, 2021 · 12 comments
Labels
enhancement ✨ New feature or request help wanted 😕 Extra attention is needed

Comments

@Helloyunho
Copy link
Member

What do you want to request?
Adding unit tests

Do you have anything to tell us about your request?
Currently, harmony doesn't have any unit tests, which can make tests so much better and reliable.

@Helloyunho Helloyunho added enhancement ✨ New feature or request help wanted 😕 Extra attention is needed labels Apr 14, 2021
@DjDeveloperr
Copy link
Member

Deno has built in test runner.

@MierenManz
Copy link
Contributor

With unit tests you mean a set of tests that use the discord api right? I think we could easily look at discordeno's implementation and use that as a reference point

@DjDeveloperr
Copy link
Member

Yes, using the Discord API itself. It won't be hard to come up with our own implementation.

@MierenManz
Copy link
Contributor

It won't be hard no. But it's handy to have a reference point. We shouldn't be leaking resources or ops like dd does tho.

@DjDeveloperr
Copy link
Member

We shouldn't be leaking resources or ops like dd does tho.

This is not possible for Discord bots unit testing. We can't reopen connection for each test, that's just a very easy to hit rate limits...

@MierenManz
Copy link
Contributor

If you create the resource before the tests and not close it in the test then it works normally. It will only show a "leak" when you close it in the test

@patgoat
Copy link

patgoat commented Jun 22, 2021

We shouldn't be leaking resources or ops like dd does tho.

This is not possible for Discord bots unit testing. We can't reopen connection for each test, that's just a very easy to hit rate limits...

Why not run the tests with github actions, and pass resources as a secret, and then a command line argument? Then load all the tests into a runner which is ran with deno run runner.ts TOKEN

Or is this just not viable? Or is this not what you mean?

@MierenManz
Copy link
Contributor

Resources are the things in the resource table. Not tokens etc.

Stuff that is in Deno.resources() like files, child processes and other things. Ops are the interactions between Rust and JavaScript. So leaking those means that some of those interactions didn't finish when the test finished.

@MierenManz
Copy link
Contributor

This is not possible for Discord bots unit testing. We can't reopen connection for each test, that's just a very easy to hit rate limits...

This is certainly possible. When trying to fix op + resource leaks in dd I found that their code only leaked them. Not the test code. So it's doable by alot

@DjDeveloperr
Copy link
Member

As this will be taken care of in v3, should this issue be closed? (can be tracked in #225 instead)

@MierenManz
Copy link
Contributor

All new functions would need their own tests. So maybe add a comment to #225 and say that all functions require a test etc.
Then I guess we can close this yeah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request help wanted 😕 Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants