Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor specs #177

Merged
merged 1 commit into from
Jan 2, 2025
Merged

Refactor specs #177

merged 1 commit into from
Jan 2, 2025

Conversation

tagliala
Copy link
Contributor

@tagliala tagliala commented Dec 3, 2024 •
edited
Loading

  • Create spec_helper and .rspec
  • Add spec/examples.txt to .gitignore and sort alphabetically.
    spec/examples.txt allows to run specs with --only-failures option
  • Add SimpleCov for code coverage. Run specs with COVERAGE env
    variable set to any value

@tagliala tagliala force-pushed the chore/refactor-specs branch from 8c8826d to 47b229a Compare December 3, 2024 16:54
@tagliala tagliala marked this pull request as draft December 3, 2024 16:55
@tagliala tagliala force-pushed the chore/refactor-specs branch from 47b229a to 4d6a693 Compare December 3, 2024 16:59
@tagliala tagliala marked this pull request as ready for review December 3, 2024 22:48
@tagliala
Copy link
Contributor Author

tagliala commented Dec 7, 2024 •
edited
Loading

@jamesmartin not immediate to review, but there are no changes to production code

Next steps:

  • Add "config teardown" to allow random spec execution
  • Increase coverage (actual: 92.7% (368 / 397))

tagliala
tagliala commented Dec 7, 2024
View reviewed changes
@tagliala tagliala force-pushed the chore/refactor-specs branch 3 times, most recently from 158fc12 to 166d863 Compare December 8, 2024 14:58
jamesmartin
jamesmartin reviewed Dec 9, 2024
View reviewed changes
Copy link
Owner

@jamesmartin jamesmartin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @tagliala. I'm not a fan of this particular RSpec pattern as it makes the individual tests less approachable. I would rather have each test be a little more verbose and reveal its intention clearly.

@tagliala
Copy link
Contributor Author

tagliala commented Dec 9, 2024

Hello,

I understand that the problem is "spec helper" and its inclusion in spec files

I've extracted #178 and #179 that should be good

Should I also create a PR for described_class ?

@jamesmartin
Copy link
Owner

I understand that the problem is "spec helper" and its inclusion in spec files

I don't think this is a problem, if it's useful.

I've extracted #178 and #179 that should be good

Both of those look good and I've merged them to master.

Should I also create a PR for described_class ?

This is the pattern I don't like ☝️. I think it's needless indirection and I prefer tests to be verbose and self-contained.

@tagliala
Copy link
Contributor Author

Ok, so I can disable that cop and proceed by re-creating the spec helper plus coverage, correct?

@tagliala tagliala force-pushed the chore/refactor-specs branch 2 times, most recently from 744d4b2 to 5fe48d0 Compare December 15, 2024 16:44
tagliala added a commit to tagliala/inline_svg that referenced this pull request Dec 15, 2024
@tagliala
Set RSpec/DescribedClass cop to explicit
69c503e 
Additionally:
- Autofix offenses
- Update RuboCop dependencies and set require to false

Ref: jamesmartin#177 (comment)
tagliala added a commit to tagliala/inline_svg that referenced this pull request Dec 15, 2024
@tagliala
Set RSpec/DescribedClass cop to explicit
c2519ab 
Additionally:
- Autofix offenses
- Update RuboCop dependencies and set require to false
- Regenerate rubocop_todo.yml

Ref: jamesmartin#177 (comment)
@tagliala tagliala mentioned this pull request Dec 15, 2024
Merged
@tagliala tagliala marked this pull request as draft December 15, 2024 16:50
@tagliala tagliala force-pushed the chore/refactor-specs branch 2 times, most recently from 8c1ee6e to c6bee29 Compare December 15, 2024 20:40
@tagliala tagliala mentioned this pull request Dec 15, 2024
Merged
@tagliala tagliala force-pushed the chore/refactor-specs branch from c6bee29 to 8ed87b2 Compare December 30, 2024 09:12
@tagliala tagliala marked this pull request as ready for review December 30, 2024 09:13
@tagliala tagliala force-pushed the chore/refactor-specs branch from 8ed87b2 to ca35aa6 Compare December 30, 2024 09:15
@tagliala tagliala requested a review from jamesmartin December 30, 2024 09:16
@tagliala
Refactor specs
7a05360 
- Create `spec_helper` and `.rspec`
- Add `spec/examples.txt` to `.gitignore` and sort alphabetically.
  `spec/examples.txt` allows to run specs with `--only-failures` option
- Add SimpleCov for code coverage. Run specs with `COVERAGE` env
  variable set to any value
@tagliala tagliala force-pushed the chore/refactor-specs branch from ca35aa6 to 7a05360 Compare December 30, 2024 09:18
@tagliala
Copy link
Contributor Author

@jamesmartin rebased, commit message change accordingly, and force pushed

This is now ready for review

jamesmartin
jamesmartin approved these changes Jan 2, 2025
View reviewed changes
Copy link
Owner

@jamesmartin jamesmartin left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks again, @tagliala! ✨

@jamesmartin jamesmartin merged commit 83556f6 into jamesmartin:master Jan 2, 2025
7 checks passed
@tagliala tagliala deleted the chore/refactor-specs branch January 2, 2025 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesmartin jamesmartin jamesmartin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tagliala @jamesmartin