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]: Add jest and write tests for scripts/extractSlideData.js #14

Closed
AhsanAyaz opened this issue Oct 27, 2023 · 9 comments · Fixed by #15
Closed

[Feature]: Add jest and write tests for scripts/extractSlideData.js #14

AhsanAyaz opened this issue Oct 27, 2023 · 9 comments · Fixed by #15
Assignees

Comments

@AhsanAyaz
Copy link
Owner

Right now, we don't have any tests in the repository. And we have the scripts/extractSlideData.js file which can have unit tests. The task is to write these unit tests.

@Rachit1313
Copy link
Contributor

I would like to work on this

@anttijankeri
Copy link

Please assign the task to me.

@Rachit1313
Copy link
Contributor

Hey @anttijankeri , Sorry but I already started working on this part and created the test functions for extractTitle. Is it possible for you work on some other issue?

@Rachit1313
Copy link
Contributor

Hi @AhsanAyaz , Could you please review and let me know if any changes are required?

@AhsanAyaz
Copy link
Owner Author

@Rachit1313 I've merged your PR but even though we use jest, the test command doesn't include jest. It still runs gulp test behind the scenes. But also has an issue with the tests in windows:
image

@AhsanAyaz
Copy link
Owner Author

@anttijankeri maybe you can have a look?

@Rachit1313
Copy link
Contributor

@Rachit1313 I've merged your PR but even though we use jest, the test command doesn't include jest. It still runs gulp test behind the scenes. But also has an issue with the tests in windows:

image

As I can confirm in the screenshot provided, The difference here is that the function returned the path with // i.e windows formatting for a path whereas I created the functions in a macOS.
This can be fixed by adding another statement of checking equal to mac version or windows output.

@anttijankeri hope this helps!
Let me know if you want me to make the changes.

@Rachit1313
Copy link
Contributor

@Rachit1313 I've merged your PR but even though we use jest, the test command doesn't include jest. It still runs gulp test behind the scenes. But also has an issue with the tests in windows:

image

For the gulp test.
That's because test script was already setup for gulp in the package.json. I wasn't sure if you wanted me to update it or not.

Let me know if you want me to change that.

@anttijankeri
Copy link

Ive fixed the problems from earlier. The windows thing may or may not work, cant test it atm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants