search

ryska / reposlist

0 stars 1 forks source link

Test suggestions #1

Open kevinmamaqi-90poe opened 9 months ago

kevinmamaqi-90poe commented 9 months ago

Hey @ryska - how are you? I hope that well.

I am Kevin Mamaqi, from 90poe, checking your test. Nice one, could run it, search and it loaded more immediately :). I have three suggestions that may improve it and I would like to share with you:

  1. Could you try and use MockProvider from Apollo client? https://www.apollographql.com/docs/react/development-testing/testing/
  2. I've seen you added codegen, but doesn't seem to generate, moreover I don't see a script in package.json. Could you review it please?
  3. Saw a non-blocking trivial linter error in the test files. Could you add/review the linter configuration?

I hope you find this comment helpful, let me know if you have any doubts.

Kind regards, Kevin

ryska commented 9 months ago

Hey! Thank you for the review :) I will look into it today or early tomorrow

ryska commented 9 months ago

Hey @kevinmamaqi-90poe, you can check my PR.

  1. Yes I added the codegen, good catch, it is reduntant in my project as im not using it. I removed it.

  2. Added lint to scripts and fixed errors.

  3. I've never used MockedProvider before, so this one is tricky for me. Im not sure if it works correctly: RepositoryTable.test.tsx - I don't understand why the data is not being loaded to the table. I tried refactoring the component to use {data} from query instead of reactive var. I tried changing cache. The Internet is not very helpful, and I feel like the answer is not as complicated as it seems

I am happy to make more changes and fix the tests but I'd need more time for this. If you know the reason for that, I'd really appreciate the help

ryska commented 9 months ago

So I changed the tests in a way that they pass (for 10 mocked repositories the table renders 10 rows). The mocks look fine, they are console.logged correctly, but they are never loaded to test table.

The previous solution is in commit #.

kevinmamaqi-90poe commented 9 months ago

Hi @ryska - sorry for long time without reply. Didn't receive/see the notification and holidays where in between 🙏 . I hope you had a good time. Regarding the code I checked the setup and seems correct, just missing the __typename for the MockedProvider to pick it. Did a push where you can see. HR will come back to you.

Let me thank you again for the update, it was well done and apollo mockProvider setup and internals can be tricky and time consuming when you haven't used them previously.