altair-graphql / altair

✨⚡️ A feature-rich GraphQL Client for all platforms.
https://altairgraphql.dev
MIT License
5.14k stars 323 forks source link

Write more tests #1717

Open imolorhe opened 3 years ago

imolorhe commented 3 years ago

While there are already lots of tests, several components are still lacking proper tests (just the one skeletal generated test). It would be nice to have tests for a lot of such components.

To make it easier to assign/attribute the tests to individual people, here's what we'll do:

Decide on what you want to write tests for The focus is on packages/altair-app directory, but writing tests for the other files/packages are also acceptable. The tests are divided by the files (so if you're picking up account-dialog component for example, then you're essentially picking up testing for the account-dialog.component.{ts|html} file as a whole, which makes sense). If you only intend to work on a part of a component, you need to make it clear so others know what isn't picked up yet. Note: For now we are not defining a minimum scope for a test task but we change this later depending on how this goes

Aim for at least 70% coverage While you can choose to write tests for anything in there, the aim is to have test coverage of at least 70% for each individual file. Some things already have tests (pretty much everything has at least one basic test but those don't count much) but the test coverage for most things are currently very low. To be clear, test coverage is just a basic metric to ensure that we are at least doing some automated checks, but it isn't the defining factor of well tested code. The tests have to be testing the actual business logic, execution flows, etc. So if a test PR is created that just aims to increase the test coverage without actually testing anything substantial, it will not be accepted. Tip: the test coverage is displayed after the tests run, so you can see the actual coverage by running yarn jest in the packages/altair-app directory. You can also just guesstimate that a file has low coverage if the test file has very little code 🙂

Add a comment about the test task Once you have decided on the things you want to test, add a comment on this issue about it. That way we can coordinate better if someone already picked it up. We'll create an issue for the task and assign the issue to you, and you can proceed from there.

Completing the task Create your PR(s) to complete the test. Once that's reviewed and merged and we confirm that the coverage is good enough, we'll close the test issue as completed. Note: if we don't do this promptly, feel free to add a comment on the issue stating that it is done to remind us

Hope this helps make it a better experience!

OrchiDorchi commented 2 years ago

I am looking forward to work on this issue. Can you provide the tests path and some sample tests? Thanks.

imolorhe commented 2 years ago

Oh thank you!

The test files are along side the source files in the same directory. For instance, the test file for environment.service.ts is environment.service.spec.ts.

Same for components as well. For example, the test for header.component.ts (which is one of the empty tests) is header.component.spec.ts. An example of a component spec with some valid tests is add-collection-query-dialog.component.spec.ts

rajpatelbot commented 1 year ago

is it still need test case if yes then please assign me

imolorhe commented 1 year ago

Feel free.

M4X1M4S commented 1 year ago

still, pending? Assign it to me.

imolorhe commented 1 year ago

To be clear, this is a rolling task and multiple people can work on different parts of it. Feel free to pick it up and I can assign it to you when you have it in progress. 🙂

ianupam001 commented 1 year ago

If it is open still may i contribute

imolorhe commented 1 year ago

Yes it is

krishh16 commented 1 year ago

I would like to contribute.

imolorhe commented 1 year ago

Feel free to do so!

utkarsh-shrivastav77 commented 1 year ago

I would like to contribute

adityadalwadi1510 commented 1 year ago

I would like to contribute.

TheRecklessDoctor commented 1 year ago

I would like to contribute

codeinate commented 1 year ago

I'm going to start working on some tests.

Tiago404 commented 1 year ago

I'm also going to work on some tests.

imolorhe commented 1 year ago

To make it easier to assign/attribute the tests to individual people, here's what we'll do:

Decide on what you want to write tests for The focus is on packages/altair-app directory, but writing tests for the other files/packages are also acceptable. The tests are divided by the files (so if you're picking up account-dialog component for example, then you're essentially picking up testing for the account-dialog.component.{ts|html} file as a whole, which makes sense). If you only intend to work on a part of a component, you need to make it clear so others know what isn't picked up yet. Note: For now we are not defining a minimum scope for a test task but we change this later depending on how this goes

Aim for at least 70% coverage While you can choose to write tests for anything in there, the aim is to have test coverage of at least 70% for each individual file. Some things already have tests (pretty much everything has at least one basic test but those don't count much) but the test coverage for most things are currently very low. To be clear, test coverage is just a basic metric to ensure that we are at least doing some automated checks, but it isn't the defining factor of well tested code. The tests have to be testing the actual business logic, execution flows, etc. So if a test PR is created that just aims to increase the test coverage without actually testing anything substantial, it will not be accepted. Tip: the test coverage is displayed after the tests run, so you can see the actual coverage by running yarn jest in the packages/altair-app directory. You can also just guesstimate that a file has low coverage if the test file has very little code 🙂

Add a comment about the test task Once you have decided on the things you want to test, add a comment on this issue about it. That way we can coordinate better if someone already picked it up. We'll create an issue for the task and assign the issue to you, and you can proceed from there.

Completing the task Create your PR(s) to complete the test. Once that's reviewed and merged and we confirm that the coverage is good enough, we'll close the test issue as completed. Note: if we don't do this promptly, feel free to add a comment on the issue stating that it is done to remind us

Hope this helps make it a better experience!

Nishant-Nagururu commented 9 months ago

I'm writing test cases for the action_bar component in Altair-app. I wrote tests for all the event emitters to make sure that those go off properly.

austinluk commented 2 months ago

Hello! I want to contribute, is there any components that need to be tested?

austinluk commented 2 months ago

Is there still anything I can test? if yes then please assign me.

Thanks!

akhilk2802 commented 2 months ago

Can i work on this ? please assign, Thanks