Developer-DAO / DAO-job-board

A job board connecting DAOs with talent.
https://devdao-job-board.vercel.app/
82 stars 34 forks source link

Add tests #137

Open carlomigueldy opened 2 years ago

carlomigueldy commented 2 years ago

TL;DR: If you just wanna do something you can add tests and we'll add that to CI.

Overview

This isn't a priority right now but if someone wants to take it and add automated tests then that would be very awesome. I have no specific tests to target but if you can come up with test cases then that's much appreciated 🤝

PBillingsby commented 2 years ago

I’m no test expert though I think we should be implementing them with the amount of data being thrown around, added with the upcoming token and amount of traffic expected to be flooding the job board.

We can put it up to vote but for me it should be considered.

carlomigueldy commented 2 years ago

I’m no test expert though I think we should be implementing them with the amount of data being thrown around, added with the upcoming token and amount of traffic expected to be flooding the job board.

We can put it up to vote but for me it should be considered.

Yo that sounds good. Can you elaborate more on this?

PBillingsby commented 2 years ago

We should be implementing at bare minimum some unit testing. With the amount of people, and possibly bots using the site, the database is at risk of bad data being passed. Writing Jest tests on stateful components like the forms will allow us, in an easier way, to check if bad data is being persisted. It will also give us a better chance at identifying bugs and where particular blocks of code are breaking.

Jest resources: https://jestjs.io/docs/getting-started https://blog.logrocket.com/testing-typescript-apps-using-jest/ https://itnext.io/testing-with-jest-in-typescript-cc1cd0095421

Basic example Jest test:

import { add } from "../src/calc";
describe("test add function", () => {
  it("should return 15 for add(10,5)", () => {
    expect(add(10, 5)).toBe(15);
  });
it("should return 5 for add(2,3)", () => {
    expect(add(2, 3)).toBe(5);
  });
});
PBillingsby commented 2 years ago

Definitely think we need some more opinions on this as I am not the most experienced tester.

carlomigueldy commented 2 years ago

Yo very awesome! Thanks for the details @PBillingsby and yeah unit tests would definitely help us guard from spams, we can write tons of scenarios that we can think of.

Definitely think we need some more opinions on this as I am not the most experienced tester.

Agreed, I've only be unit testing with Jest for like 3-4 months so I'm definitely not that experienced tester either. But I do know who's a very good at it in fact a mentor, I'll reach out to him :)

krames12 commented 2 years ago

Might I suggest that we have an accessibility tool like jest-axe and including even just the basic toHaveNoViolations() check to catch the very obvious a11y issues that people tend to miss?

carlomigueldy commented 2 years ago

Might I suggest that we have an accessibility tool like jest-axe and including even just the basic toHaveNoViolations() check to catch the very obvious a11y issues that people tend to miss?

Yo please do, we'd love to have that. :)

krames12 commented 2 years ago

Once I'm done with #115, I wouldn't mind picking this up and adding even just some basic unit tests where possible. Would it be safe to assume it would be best to submit a PR for the initial testing-library/react setup + a basic test to prove it's working and then a separate PR for each section of the project?

carlomigueldy commented 2 years ago

Once I'm done with #115, I wouldn't mind picking this up and adding even just some basic unit tests where possible. Would it be safe to assume it would be best to submit a PR for the initial testing-library/react setup + a basic test to prove it's working and then a separate PR for each section of the project?

Yes we would love to have that as well! Please do so :)

krames12 commented 2 years ago

@carlomigueldy Last question for the moment. I'm assuming this is just for unit testing and not both unit testing and end to end tests with something like Cypress, correct? If it does encompass both, I think we should make an issue to discuss what type of testing would need to be done there that isn't covered with unit tests.

frankTurtle commented 2 years ago

At a bare minimum, there needs to be unit tests. Integration tests are always helpful, too, ofc.

4gnle commented 2 years ago

I'd love to have tests for the project, but we probably need to add them once we have the MVP or before launching it, so adding a for-later tag to this