fix: allow all combinations of describe, it and test to skip modifier

[wellwelwel]

Fixes: b756605#r146279295

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.07%. Comparing base (228177f) to head (fc8b4c7).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #746   +/-   ##
=======================================
  Coverage   99.06%   99.07%           
=======================================
  Files          36       36           
  Lines        1720     1728    +8     
  Branches       10       10           
=======================================
+ Hits         1704     1712    +8     
  Misses          8        8           
  Partials        8        8           

Flag	Coverage Δ
linux	98.95% <100.00%> (+<0.01%)	⬆️
osx	99.01% <100.00%> (+<0.01%)	⬆️
windows	98.95% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mrazauskas]

Nice! Isn't it that .todo has similar issue?

[wellwelwel]

Thanks, @mrazauskas.

Isn't it that .todo has similar issue?

I don't know if this will make sense in English, but a "to-do" without a "do" seems to be just an alternative name for the skip method.

My idea/approach for each one is:

.skip

• A debugging helper
• Message is optional
• Callback is required

.todo

• A planning helper for incomplete or future tests
• Message is required
• Callback is optional

[mrazauskas]

That's right. It is just a skipped test, but semantically it is not skipped, but simply not yet (properly) implemented.

[mrazauskas]

Also if Poku allows tests without messages, I think the planning helper must allow those too. Why not?

[wellwelwel]

Poku allows tests without messages

I think I get your point now. My problem with planning without a message is that it would cease to be a plan, for example:

• This is a plan:

test.todo("Understand why the loop doesn't finish", () => {
  let i = 0;
  while (i >= 0) i++;
);

• Same test, but this one wouldn't be a plan, just an ignored test:

test.skip(() => {
  let i = 0;
  while (i >= 0) i++;
);

Both will do exactly the same thing in practice (skip the tests), but one shows that there is planning behind the "improperly implemented" test, the other just ignores it. The real difference is in the final logs.

That said, I'd have no problem making .todo an alias for skip. If I understand right, the idea is that any test should be able to use a modifier without requiring any change to it.

[mrazauskas]

Looking at your example I think: why .todo is needed at all? Use .skip instead. Optional callback is not really so much useful, one can use a comment too:

// TODO Why this loop does not work?
test.skip(() => {
  let i = 0;
  while (i >= 0) i++;
);

---

What if someone is building up a test without a message (since that is allowed):

test.todo(() => {
  someCall();

  // assert this and that

  // TODO does not work yet, have to figure it out
  someOtherCall();

  // assert this and that
);

Here .todo can be replace with .skip, but semantics are different.