8000 docs(ruby): add multiple expectations guide by kfitzpatrick · Pull Request #168 · habit-hooks/habit-hooks · GitHub
[go: up one dir, main page]

Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/smell-vocabulary.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ exits 0. The mapper config can override it per project.
| `deep-nesting` | Deep nesting | enforced |
| `oversized-file` | Oversized file | enforced |
| `oversized-block` | Oversized block | enforced |
| `multiple-expectations` | Multiple expectations | enforced |
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I'm not sure about is this line here. I suspect this is not a warning that will be in most tools and is ruby specific. It's not bad practice, but it's BDD specific.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the smell, I'm not sure about the name for now. However I would definitely move this one to be suggested instead of enforced. It's too subjective.

| `unused-variable` | Unused variable | enforced |
| `loose-equality` | Loose equality | enforced |
| `var-declaration` | `var` declaration | enforced |
Expand Down Expand Up @@ -189,6 +190,7 @@ catalogue is shared, only the plugin's sensors differ).
| `rubocop:Metrics/ParameterLists` | `too-many-parameters` |
| `rubocop:Metrics/MethodLength` | `oversized-function` |
| `rubocop:Metrics/BlockLength` | `oversized-block` |
| `rubocop:RSpec/MultipleExpectations` | `multiple-expectations` |
| `rubocop:Metrics/CyclomaticComplexity` | `high-complexity` |
| `rubocop:Metrics/PerceivedComplexity` | `high-complexity` |
| `rubocop:Metrics/AbcSize` | `high-complexity` |
Expand Down
46 changes: 46 additions & 0 deletions plugins/ruby/src/habit_hooks_ruby/guides/multiple-expectations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
Keep each example focused on one behavior and one clear reason to fail.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When creating guides, where possible we should aim for generic wording that works for any language, and move the guide to generic. Example code is discouraged in prompts for the same reason.

Here is an alternative draft for this prompt:

The tests marked bellow have multiple assert/expectation statements in them. This can indicate that the test is testing multiple things, and may break for multiple reasons. When that happens it makes identifying the cause of the test failure harder. 

Please follow these steps when evaluating these steps:
- Judge if the expectations are testing one concept using multiple statements, or they should rather be in two separate tests.
- If the expectations are testing one concept, refactor by naming that concept, and creating a custom assertion/expectation or a helper method to wrap the statements in a single statement
- If the expectations are testing multiple concepts split the test into multiple smaller tests. Extract helper methods to avoid duplication. 

Split unrelated expectations into separate examples, sharing setup through
`before`, helpers, or `let` where useful.

Bad — unrelated behavior is bundled into one example:

```ruby
it "creates and announces the account" do
account = create_account

expect(account).to be_persisted
expect(account.plan).to eq("free")
expect(Mailer).to have_received(:welcome).with(account)
end
```

Good — each behavior has its own example:

```ruby
subject(:create_account) { described_class.call }

it "persists the account" do
expect(create_account).to be_persisted
end

it "assigns the free plan" do
expect(create_account.plan).to eq("free")
end

it "sends the welcome email" do
account = create_account

expect(Mailer).to have_received(:welcome).with(account)
end
```

Multiple expectations can be appropriate when they jointly describe one
inseparable outcome. Use judgment: improve the test’s focus and diagnostic value;
do not combine assertions, add `aggregate_failures`, or disable the cop merely to
silence RuboCop.

Locations reported:

{% for issue in issues -%}
{{ issue.details.file }}:{{ issue.details.line }} — {{ issue.details.message }}
{% endfor %}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"Metrics/ParameterLists": "too-many-parameters",
"Metrics/MethodLength": "oversized-function",
"Metrics/BlockLength": "oversized-block",
"RSpec/MultipleExpectations": "multiple-expectations",
"Metrics/CyclomaticComplexity": "high-complexity",
"Metrics/PerceivedComplexity": "high-complexity",
"Metrics/AbcSize": "high-complexity",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ def test_every_mapped_cop_reaches_its_smell(cop: str) -> None:
assert finding["issues"][0]["details"]["source"] == f"rubocop:{cop}"


def test_multiple_expectations_reaches_its_guide_smell() -> None:
[finding] = findings([_entry("RSpec/MultipleExpectations")])

assert finding["smell"] == "multiple-expectations"
assert finding["issues"][0]["details"]["source"] == "rubocop:RSpec/MultipleExpectations"


def test_findings_are_grouped_by_smell_and_sorted() -> None:
"""One finding per smell whatever order the offences arrive in. The output
is compared byte for byte by the spec cases, so it has to be deterministic."""
Expand Down
Loading
0