Add requirement for component tests #18015
Conversation
|
perhaps this is just something we can automate with schemas? |
Jenkins BuildsClick to see older builds (4)
|
Once a components has a function schema, what we consider "empty" will need to fulfill the schema. The minimum set of args may be an empty map, but it may not be as well. Automating this is tricky, unless it's the obvious empty map case (but then we don't need malli for that). The other technique is with generative tests, which often are able to shrink the valid set of args to a minimum, so they often find the empty cases (empty strings, nil values, empty maps, and so on). Sadly, component tests are considerably slower than normal unit tests, so I think generative tests in component tests need further experimentation. All this to say, I'm not sure how much malli can help us reinforce or automate the generation of default test cases because malli doesn't know how to magically shrink to the default case. It's worth exploring @J-Son89. |
|
Okay, let's add to the guidelines for now and we can do some experimentation with it and come back to this later on. |
|
btw, @ilmotta raised a good point before. For docs changes we should add many reviewers and share in the channel for everyone to see. More likely that everyone becomes aware of the updates then 👌 |
doc/new-guidelines.md
Outdated
| @@ -687,6 +687,17 @@ actually subscribing to them, so reframe's signal graph gets validated too. | |||
| (is (= expected (rf/sub [sub-name]))))) | |||
| ``` | |||
|
|
|||
| #### Component tests | |||
There was a problem hiding this comment.
perhaps a better location for this is in the quo directory - there is a readMe there?
https://github.com/status-im/status-mobile/blob/develop/src/quo/README.md
There was a problem hiding this comment.
@J-Son89, not sure about this. I think that document tree in the main readme file is more convenient, because it is likely to be read. The chance that people will find this piece of information in some other directory is much lower. For example I didn't even know that quo directory has a readme... just never noticed
There was a problem hiding this comment.
To me both places seem okay to have this information,
I'd lean more towards Quo readme because this guideline specifically is related to quo component tests and would be good to maybe have that linked somehow here in guidelines document.
No strong opinions however :)
There was a problem hiding this comment.
I think dispersing the READMEs in top-level directories is a common practice, but easy to overlook. Good point @vkjr, maybe we should also have a top-level table of contents on the main guidelines file pointing to secondary READMEs.
There was a problem hiding this comment.
Moved the text about no-props test to quo/readme, added link to that readme in guidelines, under "project structure" section, wdyt?
vkjr
requested review from
ilmotta,
rasom,
flexsurfer,
siddarthkay,
ulisesmac,
cammellos,
OmarBasem and
mmilad75
vkjr
force-pushed
the
guidelines-component-tests
branch
from
9118173
to
42409a0
Compare
Summary
Added requirement to have a component test without props. To ensure components don't crash in such case
status: ready