test setup #74 #75

AhmedEldessouki · 2021-02-01T17:21:14Z

No description provided.

AhmedEldessouki · 2021-02-01T17:21:43Z

resolves #74

removed The AllRaid assertions from App.test and moved them to AllRaids.test. wrote ViewRaid Tests to cover all sorting the the Total view

nobrayner

Some general things:

I don't like all the mocking, but I am unsure how to avoid it at this point
I would prefer component/route tests to be collocated

package.json

src/__tests__/AllRaids.test.tsx

src/__tests__/App.test.tsx

src/__tests__/Raids.test.tsx

src/tests/data/raidsData.ts

Co-authored-by: Braydon Hall <[email protected]>

estEnviroment node cause the snowpack build to fail

package.json

src/__tests__/firestore.ts

nobrayner · 2021-02-03T19:39:55Z

Thanks for working on this - you are doing some great work 🎊

AhmedEldessouki · 2021-02-03T19:52:35Z

Thanks for working on this - you are doing some great work 🎊

Thanks 🙂

AhmedEldessouki · 2021-02-05T13:43:47Z

package.json

-    "test": "jest --watch --FIRESTORE_EMULATOR_HOST=8080",
-    "test:ci": "jest",
+    "test": "jest --watch",
+    "test:ci": "jest --findRelatedTests",


--findRelatedTests forces jest from running cypress's tests (lint-stage for some reason forces jest to run cypresses tests)🙂

nobrayner · 2021-02-09T21:06:37Z

So just some reminders for myself, and to mention a few things:

I don't like all the mocking, but I am unsure how to avoid it at this point

This can be fixed by making the test command use firebase emulators:exec --only firestore "jest --watch" for example. This will start up the firestore emulator, run jest (with full watch mode stuff working), then when you quit jest, the emulators cleanly shutdown. This then allows us to avoid mocking completely.

You can see how I got this working in Guild Scrivener:
https://github.com/OpenSourceRaidGuild/Guild-Scrivener/blob/main/package.json
https://github.com/OpenSourceRaidGuild/Guild-Scrivener/tree/main/src/testUtils
https://github.com/OpenSourceRaidGuild/Guild-Scrivener/blob/main/src/setupTests.ts

I would prefer component/route tests to be collocated

Looks like the tests are still hanging out under the __tests__. Honestly, the original intention was to have a tests folder per place that needed testing.... But that seems overly complicated now, and should probably just be the test files sitting right next to the things they test.

AhmedEldessouki · 2021-02-10T13:32:10Z

This can be fixed by making the test command use firebase emulators:exec --only firestore "jest --watch" for example. This will start up the firestore emulator, run jest (with full watch mode stuff working), then when you quit jest, the emulators cleanly shutdown. This then allows us to avoid mocking completely.

But isn't the way you started the emulators wouldn't import the dev-data file. Are intending you fully create a collection before running a test?

You can see how I got this working in Guild Scrivener:
https://github.com/OpenSourceRaidGuild/Guild-Scrivener/blob/main/package.json
😍 pretty smart i must say. why doesn't it work if you ran it like this "run-s exec:firestore test" 🤓

https://github.com/OpenSourceRaidGuild/Guild-Scrivener/tree/main/src/testUtils
🤯🤯 no idea whats going on except for most of faker usage

https://github.com/OpenSourceRaidGuild/Guild-Scrivener/blob/main/src/setupTests.ts
when i started testing my app (which is also) using firebase. I used to get my API like this [...API_KEY]. so how did dotenv.config(); fix it? ... i used to think that Kent did something so people like me wouldn't do the mistake of testing using the same backend the app uses.

I would prefer component/route tests to be collocated
+1
@JacobMGEvans what do you think? ⬆⬆

Looks like the tests are still hanging out under the __tests__. Honestly, the original intention was to have a tests folder per place that needed testing.... But that seems overly complicated now, and should probably just be the test files sitting right next to the things they test.
are you thinking components/__tests__ or components/{whatever}/__tests__ ? 🤔

nobrayner

@AhmedEldessouki I'll answer some of your questions below 😄

why doesn't it work if you ran it like this run-s exec:firestore test

~~If you meant start:firestore - yes, that would work. But why should I bother about adding in another dependency, when the firebase emulator supports the usecase we have out of the box?~~ Sorry, this would NOT work. run-s runs things sequentially, and the firestore emulator just runs until stopped - so it would never run the test script.

🤯🤯 no idea whats going on except for most of faker usage

Ask any questions in Discord, and I'll do my best to explain!

are you thinking components/tests or components/{whatever}/tests ? thinking

Neither, actually. That's what I was originally thinking. Now I believe doing something like components/loadingScreen/loadingScreen.test.ts <-- this will place it right next to the file it is testing.

nobrayner · 2021-02-10T20:24:50Z

package.json

    "start:dev": "snowpack dev",
    "start:firestore": "firebase emulators:start --import=./devData --only firestore",
    "build": "snowpack build",
-    "test": "jest --watch",
+    "test": "jest --watch --FIRESTORE_EMULATOR_HOST=8080",


when i started testing my app (which is also) using firebase. I used to get my API like this [...API_KEY]. so how did dotenv.config(); fix it? ... i used to think that Kent did something so people like me wouldn't do the mistake of testing using the same backend the app uses.

Dotenv just imported whatever I had in my .env file, as without those, it would fall over. This also meant that I don't have to worry about var being added or removed. It becomes extremely(!!!!) important to make sure you aren't "leaking" anything out to the real world, however. In Scrivener, I am managing this by using MSW with a catchall:

rest.get('*', (req, res, ctx) => { const warning = `${req.url} has not been mocked yet... Maybe now is a good time to do that`; console.warn(warning); return res(ctx.json(warning)); }),

And then firestore has this really nice feature of the FIRESTORE_EMULATOR_HOST env var, which will make it point to the emulator instead of real firebase.

Sorry, this would NOT work. run-s runs things sequentially, and the firestore emulator just runs until stopped - so it would never run the test script.

I thought it will run firestore emulator then jest. but even run-p didn't work even thu it worked with start command.

components/loadingScreen/loadingScreen.test.ts <-- this will place it right next to the file it is testing.

this will surely be a lot cleaner. I will do the changes.
@JacobMGEvans is it okay to do so?

i see now. that cleared alot of things for me. 🙏🏼 thanks 🙂
using the emulator is way better than using MSW to fake the requests. because firebase url are pretty damn big.

Suggested change

"test": "jest --watch --FIRESTORE_EMULATOR_HOST=8080",

"test": "jest --watch",

I dont think that the FIRESTORE_EMULATOR_HOST var works here in this line. when i added it, it was for the test env to pick up on it but the way you wrote the script and configured the env var bypassed its use case.

@AhmedEldessouki the way you set it here doesn't set an env var. You will have to use something like cross-env and set it first - see Guild Scrivener for how I setup a generic "execute firestore" command that everything else hooks off of for a solid example.

thanks for pointing that out 🙂 I will look for it 😁😁

Cant work around this. int the Guild Scrivener you are using Node env. do you think it's possible for @firebase/testing to work in Jsdom env?
Error: FIRESTORE (8.2.4) INTERNAL ASSERTION FAILED: Unexpected state

I ran into this problem before when i was testing firestore-rules (Node env). after i worked around it, jsdom tests was being unexpected and then i removed firestore-rules test

nobrayner · 2021-02-10T20:37:31Z

src/__tests__/Feedback.test.tsx

+  jest.doMock('../utils/useDocument', () =>
+    jest.fn().mockReturnValue(fetchedDocumentData),
+  )
+  jest.doMock('../utils/useCollection', () =>
+    jest.fn().mockReturnValue(fetchedCollectionData),
+  )
+  jest.spyOn(console, 'log')


When we use the emulators, we don't have to mock anything because we are just interfacing with the emulator instead of the real thing - yes you still have to "seed" the data.

This can be done by just importing from the devData dir for now, as the website is really read-only.

When we use the emulators, we don't have to mock anything

totally agree. i tried it before mocking but when it failed i when with mocking. i will do remove the mocks.

JacobMGEvans · 2021-02-11T01:36:12Z

src/components/feedback/index.tsx

+    // console.log({
+    //   name: name.value,
+    //   raidName: raidName.value,
+    //   rate: rate.value,
+    //   description: description.value,
+    // })


Clean up later

JacobMGEvans · 2021-02-11T01:37:37Z

src/tests/testUtils.ts

+  )
+}
+
+export * from '@testing-library/react'


Why is this being exported?

I believe it is so there is a single import { xyz } from '../path/to/testUtils' instead of importing things from '@testing-library/react' itself AND 'testUtils'.

JacobMGEvans · 2021-02-13T20:19:06Z

Replaced By #83

test setup OpenSourceRaidGuild#74

4f8caad

AhmedEldessouki added 3 commits February 1, 2021 22:52

Tests

8572c31

removed The AllRaid assertions from App.test and moved them to AllRaids.test. wrote ViewRaid Tests to cover all sorting the the Total view

replaced most getByRole to getByText and getByAltText

5d8e8ad

replaced most getByRole img to getByAltText

a6f6d52

nobrayner requested changes Feb 1, 2021

View reviewed changes

nobrayner linked an issue Feb 1, 2021 that may be closed by this pull request

Add Tests #74

Open

nobrayner assigned AhmedEldessouki Feb 1, 2021

nobrayner added the Status: Revision Needed 💭 Was your sword bent or shield cracked!? It's ok we got you, we defeat or are defeated AS ONE!!! label Feb 1, 2021

AhmedEldessouki and others added 5 commits February 2, 2021 00:36

Update src/__tests__/App.test.tsx

0e310e9

Co-authored-by: Braydon Hall <[email protected]>

tests suggested changes

1a2b21c

Feedback test

eaff0ec

rules tests

76d16cf

removed firebase/rules tests

430711c

estEnviroment node cause the snowpack build to fail

nobrayner requested changes Feb 3, 2021

View reviewed changes

package.json Show resolved Hide resolved

src/__tests__/firestore.ts Outdated Show resolved Hide resolved

JacobMGEvans added Status: In Progress 🚧 Currently taking out the trash mobs or cleaning up the dungeon Type: Test Your Mettle 🧪 Can your code hold up to the brutal crucible of tests!? labels Feb 3, 2021

JacobMGEvans and others added 2 commits February 3, 2021 15:25

Merge branch 'main' into ahmedeldessouki

e4044ed

Merge branch 'main' into ahmedeldessouki

45b13a3

JacobMGEvans added the Status: Pending ⏳ label Feb 4, 2021

AhmedEldessouki commented Feb 5, 2021

View reviewed changes

AhmedEldessouki requested a review from nobrayner February 5, 2021 13:43

JacobMGEvans added the CTA: Help Wanted CALL TO ACTION: Rally to Aid a fellow Raid Member!!! label Feb 5, 2021

AhmedEldessouki force-pushed the ahmedeldessouki branch from f31837b to 45b13a3 Compare February 10, 2021 10:13

nobrayner approved these changes Feb 10, 2021

View reviewed changes

nobrayner requested a review from JacobMGEvans February 10, 2021 20:45

JacobMGEvans reviewed Feb 11, 2021

View reviewed changes

JacobMGEvans approved these changes Feb 11, 2021

View reviewed changes

Merge branch 'main' into ahmedeldessouki

c7afdc5

JacobMGEvans mentioned this pull request Feb 13, 2021

Ahmedeldessouki #83

Merged

JacobMGEvans closed this Feb 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test setup #74 #75

test setup #74 #75

AhmedEldessouki commented Feb 1, 2021

AhmedEldessouki commented Feb 1, 2021

nobrayner left a comment •

edited

Loading

nobrayner commented Feb 3, 2021

AhmedEldessouki commented Feb 3, 2021

AhmedEldessouki Feb 5, 2021 •

edited

Loading

nobrayner commented Feb 9, 2021

AhmedEldessouki commented Feb 10, 2021

nobrayner left a comment •

edited

Loading

nobrayner Feb 10, 2021

AhmedEldessouki Feb 11, 2021 •

edited

Loading

AhmedEldessouki Feb 11, 2021

nobrayner Feb 11, 2021

AhmedEldessouki Feb 11, 2021

AhmedEldessouki Feb 11, 2021

nobrayner Feb 10, 2021

AhmedEldessouki Feb 11, 2021

JacobMGEvans Feb 11, 2021

JacobMGEvans Feb 11, 2021

nobrayner Feb 11, 2021 •

edited

Loading

JacobMGEvans commented Feb 13, 2021

	"test": "jest --watch --FIRESTORE_EMULATOR_HOST=8080",
	"test": "jest --watch",

test setup #74 #75

test setup #74 #75

Conversation

AhmedEldessouki commented Feb 1, 2021

AhmedEldessouki commented Feb 1, 2021

nobrayner left a comment • edited Loading

Choose a reason for hiding this comment

nobrayner commented Feb 3, 2021

AhmedEldessouki commented Feb 3, 2021

AhmedEldessouki Feb 5, 2021 • edited Loading

Choose a reason for hiding this comment

nobrayner commented Feb 9, 2021

AhmedEldessouki commented Feb 10, 2021

nobrayner left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AhmedEldessouki Feb 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nobrayner Feb 11, 2021 • edited Loading

Choose a reason for hiding this comment

JacobMGEvans commented Feb 13, 2021

nobrayner left a comment •

edited

Loading

AhmedEldessouki Feb 5, 2021 •

edited

Loading

nobrayner left a comment •

edited

Loading

AhmedEldessouki Feb 11, 2021 •

edited

Loading

nobrayner Feb 11, 2021 •

edited

Loading