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

KImg: Introduce testing #512

Closed
EshaanAgg opened this issue Dec 24, 2023 · 8 comments · Fixed by #518
Closed

KImg: Introduce testing #512

EshaanAgg opened this issue Dec 24, 2023 · 8 comments · Fixed by #518
Assignees

Comments

@EshaanAgg
Copy link
Contributor

Product

KDS

Desired behavior

The recently introduced KImg component must have some associated tests to ensure that future changes to the library do not break the existing features.

Current behavior

There are no tests associated with the KImg component.

Add labels

testing

@EshaanAgg
Copy link
Contributor Author

I want to work on this issue!

@MisRob
Copy link
Member

MisRob commented Jan 9, 2024

Hi, @EshaanAgg, I'm sorry for delayed response, most of us were away during holidays.

I think you don't need to be testing purely visual things, but if you could introduce a basic test suite whenever it seems relevant to you, that'd be wonderful. From the other issue you opened in regards to testing, it seems to me you have a good idea about general principles of testing, so I think you're welcome to find areas to test.

@MisRob
Copy link
Member

MisRob commented Jan 9, 2024

@EshaanAgg There may be some other components to test in KDS, so you're welcome to look into that after this issue, if you'd be interested.

@EshaanAgg EshaanAgg mentioned this issue Jan 10, 2024
9 tasks
@EshaanAgg
Copy link
Contributor Author

@EshaanAgg There may be some other components to test in KDS, so you're welcome to look into that after this issue if you'd be interested.

Sure! I would love it if you could review the KImg PR to ensure that we are on the same page regarding the testing philosophy in the project, and then I would love to continue opening PRs for other components as and when I get the time.

@MisRob
Copy link
Member

MisRob commented Jan 10, 2024

That sounds great, thank you, @EshaanAgg.

I have a few PRs in my queue, so please give me a few days.

@MisRob
Copy link
Member

MisRob commented Jan 18, 2024

@EshaanAgg

Sure! I would love it if you could review the KImg PR to ensure that we are on the same page regarding the testing philosophy in the project, and then I would love to continue opening PRs for other components as and when I get the time.

I think you can feel free to continue if you'd like to, your first test suite is very good. We have tests for some components, but I wouldn't be surprised if you discovered some more that still need them.

I also remember #510. I just need to find some time to read through it and chat about it with the team. I can definitely say that it looks interesting and it might solve some issues I am concerned about too. I will try to have a look at it next week, hopefully, but don't know for sure. So if you wouldn't want to wait, I think it'd be best to write tests with the tools we currently use. It's up to you.

@EshaanAgg
Copy link
Contributor Author

Thanks for the kind review @MisRob! I'm running a bit busy these days, but I'd love to swing at the other components whenever possible. As for the migration to testing library, I would love to hear how the team reacts to the same!

@MisRob
Copy link
Member

MisRob commented Jan 18, 2024

Sure, no rush at all, this is nothing pressing. We probably wouldn't get to it ourselves at all, so take your time and come back at any time. Thanks!

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 a pull request may close this issue.

2 participants