chore: add tests for disabling blocks #7279
Conversation
github-actions
bot
added
PR: chore
BeksOmega
force-pushed
the
test/disabling
branch
from
a043a1f
to
e1306f9
Compare
| let isCollapsed = await getIsCollapsed(browser, blockId); | ||
| chai.assert.isFalse(isCollapsed); |
There was a problem hiding this comment.
Why not check it's not collapsed before collappsing?
(And same below for disabling.)
There was a problem hiding this comment.
Unit testing best practice is to only include a single set of assertions in a given test. It makes your test more readable, because it's clear what's actually under test. If you have multiple assertions, you're either testing setup (i.e. something you don't actually need to be verifying / should be verified separately), or you're testing multiple behaviors, in which case they should be split up into multiple tests.
There was a problem hiding this comment.
I'm joining this conversation because Beka referred to it in comments on my recent PR. Personally, I don't see a lot of value in treating these as unit tests with a single assertion per test. The manual tests that they are modeled on do a sequence of actions, checking for failure at each step, in order to test that a workflow is working. While we can write these tests so that they are completely separated from each other, it seems to me this creates a lot of extra work and duplication of setup.
Personally, I would rather be able to describe the tests as a set of actions by a user, and have debugging involve stepping through those actions manually to find which one failed--even if that means that an individual test does not point you directly to the specific line that failed. For my right-click tests this could also take the form of doing all of the actions (collapse, expand, disable, enable, add comment, remove comment) one after another in a single test instead of doing each as a separate test with separate creation of the block under test.
There was a problem hiding this comment.
I'm not sure where this fits into formal models of testing, although in my head this is end-to-end testing.
BeksOmega
force-pushed
the
test/disabling
branch
from
020f906
to
2fc38fc
Compare
|
@cpcallen ready for re-review =) |
BeksOmega
force-pushed
the
test/disabling
branch
from
2fc38fc
to
c60f499
Compare
The basics
npm run formatandnpm run lintThe details
Resolves
Fixes #7277
Proposed Changes
Adds tests for disabling blocks via the right click context menu.
Reason for Changes
Testing is great!
Test Coverage
This is tests :P
Additional Information
N/A