SolidOS / solid-ui

User Interface widgets and utilities for Solid
https://solidos.github.io/solid-ui/dist/solid-ui.js
MIT License
148 stars 41 forks source link

Need to call clearStore several times before it works #265

Closed michielbdejong closed 4 years ago

michielbdejong commented 4 years ago

I made it a while loop and added a console.log in the commentField branch, and you see it getting called several times.

michielbdejong commented 4 years ago

See https://github.com/solid/solid-ui/compare/commentField?expand=1#diff-045df7f6173b4ac140fc7bd10e3554e3R7

megoth commented 4 years ago

This was unexpected behavior. Could it have something to do with the store being global and tests running atop each other? Maybe something we want to check with Tim.

In any case, might be (another) reason to avoid using a global store.

michielbdejong commented 4 years ago

Yeah, must be related to how Jest runs; @Vinnl any idea what we could do about this?

Vinnl commented 4 years ago

@michielbdejong The diff link you posted earlier was to master, which has now merged those changed resulting in an empty diff, so I'm missing some context here.

That said: yes, if we're using rdflib's store in the tests, then you should probably make sure it gets emptied before every test. (And agreed with Arne: it really shouldn't be global.) This probably means adding a beforeEach() to every test file that does that, though perhaps you might be able to set it up once globally using setupFilesAfterEnv (but I haven't tried that).

michielbdejong commented 4 years ago

Ah, actually no, this is unrelated to running tests in parallel. Look, it also happens if you run only one test:

~/gh/solid/solid-ui $ ./node_modules/.bin/jest test/unit/widgets/forms/comment.test.ts 
 PASS  test/unit/widgets/forms/comment.test.ts
  Comment
    ✓ runs (11ms)
    ○ skipped exists
    ○ skipped deals with missing content
    ○ skipped deals with missing field params in non-bottom field type
    ○ skipped deals with missing container
    ○ skipped deals with custom style

  console.log src/debug.ts:4
    Unique quadstore initialized.

  console.log test/unit/helpers/clearStore.ts:7
    Clearing store...

  console.log test/unit/helpers/clearStore.ts:7
    Clearing store...

Test Suites: 1 passed, 1 total
Tests:       5 skipped, 1 passed, 6 total
Snapshots:   1 passed, 1 total
Time:        1.176s, estimated 2s
Ran all test suites matching /test\/unit\/widgets\/forms\/comment.test.ts/i.

I just tested this on the no-console branch with a single test enabled (it.only).

You see that https://github.com/solid/solid-ui/blob/ac8d4ca1fc9bbf14275ace77bb78d59b0cfa852b/test/unit/helpers/clearStore.ts#L7 gets called twice instead of once. Why does store.statements.forEach(store.remove.bind(store)) not remove all statements?

I'll try out what happens if I wait for 1000ms before and/or after it.

michielbdejong commented 4 years ago

Sussed it :) See #270.

megoth commented 4 years ago

Ah, it needed a copy since it was running through an array that was being manipulated while being looped. Nice find!