Adds tests for the ExperimentDetails page #404
Conversation
rileyjbauer
force-pushed
the
experiment-details-tests
branch
from
c190859
to
cf39fcd
Compare
|
|
||
| it('uses an empty string if the experiment has no description', async () => { | ||
| const experiment = newMockExperiment(); | ||
| experiment.description = undefined; |
There was a problem hiding this comment.
nit: I think what you want here is delete experiment.description.
There was a problem hiding this comment.
Done.
just to avoid = undefined ?
There was a problem hiding this comment.
This is very nit-picky in this context, but it's a technical difference. If you set the field to undefined, it still exists on the object, and things like expect(x).not.toHaveProperty for example will fail. If we have logic that does something like if 'description' in experiment or if experiment.hasOwnProperty(description), the test won't be testing that.
Again, very nit-picky. :)
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yebrahim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yebrahim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* add cleanup_target_http_proxies * ignore too-many-lines
The only real functional changes are adding more granularity to the try/catch in
ExperimentDetailsand callingrefreshon therunlistRefas part ofload()rather than as a callback forsetStatesince it doesn't depend on state.This change is