[Rule Update] padding-around-expect-groups configuration/default

[dangreenisrael]

[Rule Update] padding-around-expect-groups configuration/default

[benkimpel]

Hey @ryanwilsonperkin and @hockeybuggy!

It's funny, but the expect behaviour we implemented is exactly what I needed for my team, but it's good to see some feedback on it in practice from people that I do not boss around. Let me share the thinking that went into it and then let me know if that changes any minds. ;^) It's ok if it doesn't, formatting is subjective and personal.

The goal was to support as best we can the Arrange Act Assert pattern of writing tests, wherein the blank line is an important part of delimiting the steps. This plugin can't get us all the way there by any means, but it can handle maybe 90% of the separation between act and assert. There are other ESLint rules (padding-line-between-statements which this rule is based on) that can add a padding line between variable declarations at the top of a scope so we can get closer still... but a full programmatic implementation of Arrange Act Assert is probably not possible. Still, 90% reliable separation between Act and Assert seemed like a win for me.

So that was the goal, but as noted above formatting is subjective. For an opinionated formatting plugin like this one the best we can do is pick something that most people like as the default and enforce it while providing a few escape hatches along the way. So long story short... It's all good with me to take it out of recommended if people feel strongly about it. I'd vote for removing it from recommended over making it a configuration option of some kind.

@ryanwilsonperkin as a workaround for now you'll want to list the rules individually, excluding the expect of course.

[hockeybuggy]

I am a big fan of using whitespace as a way to communicate different sections of the tests. On my best days I prefer:

it("should do that thing", () => {
// [setup]

// [preconditions]

// exercise subject of test

// postconditions

// [cleanup]
})

The think that I don't like with this rule is that it's assuming that the only thing in the preconditions and postconditions sections are expect calls. As shown in @ryanwilsonperkin's examples it's sometimes prudent to alias something by assigning to a variable.

I do think that this rule may be something that some people would like to have access to, but think that it should be excluded from the recommend preset.

[dangreenisrael]

@benkimpel thoughts?

[dangreenisrael]

I'm +1 to taking this our of the recommended config. Especially since @benkimpel is working on an 'all' rule.