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

Don't comment out tests #361

Open
6 of 11 tasks
IndrajeetPatil opened this issue Mar 27, 2023 · 4 comments
Open
6 of 11 tasks

Don't comment out tests #361

IndrajeetPatil opened this issue Mar 27, 2023 · 4 comments
Assignees
@bwiernik @DominiqueMakowski @IndrajeetPatil @rempsyc @strengejacke @mattansb @etiennebacher
Labels
Code Style 👩‍💻 Core Packages 📦 Discussion and planning about core packages of easystats

Comments

@IndrajeetPatil
Copy link
Member

IndrajeetPatil commented Mar 27, 2023 •
edited

There is nothing that makes other developers lose faith in our software than discovering that the commented out tests, since it's basically a mechanism to sweep known bugs under the rug.

Unfortunately, we are doing this a lot in some repos, and we need to undo this.

There are a few alternatives to commenting out tests:

  • Uncomment tests and let the build fail until these tests are fixed. The failing builds will act as the fire under our feet to motivate a quick fix.
  • Make a PR and uncomment tests in that PR, and keep working in that PR until the tests pass. The default branch builds remain green in this case.
  • Remove the tests because they are redundant.
  • Skip them in whatever contexts that are currently posing problems for them (e.g. M1mac).
  • Keep them commented but add a TODO, FIXME, etc. marker to remind us to fix them.

You can check the list below when there are no more commented out tests, which might already be the case for a few repos.

Progress tracker

  • insight
  • modelbased
  • bayestestR
  • effectsize
  • parameters
  • performance
  • correlation
  • report
  • see
  • datawizard
  • easystats
@IndrajeetPatil IndrajeetPatil added Core Packages 📦 Discussion and planning about core packages of easystats Code Style 👩‍💻 labels Mar 27, 2023
@IndrajeetPatil
Copy link
Member Author

If you want to check for commented out tests in your package, one option is to run the following:

lintr::lint_dir("tests", linters = lintr::commented_code_linter())

@IndrajeetPatil
Copy link
Member Author

So {effectsize} is not always perfect.

image

IndrajeetPatil added a commit to easystats/report that referenced this issue Mar 27, 2023
@IndrajeetPatil
Uncomment report_sample() tests
542cb6f 
easystats/easystats#361
@IndrajeetPatil IndrajeetPatil mentioned this issue Mar 27, 2023
Merged
IndrajeetPatil referenced this issue in easystats/see Mar 27, 2023
@IndrajeetPatil
uncomment test
e2e7b29
@mattansb
Copy link
Member

effectsize has only one such test of functions that have been moved to parameters! 🏆🏆🏆

IndrajeetPatil added a commit to easystats/effectsize that referenced this issue Mar 28, 2023
@IndrajeetPatil
Remove commented code in tests
83ca4a5 
cf. easystats/easystats#361
@IndrajeetPatil IndrajeetPatil mentioned this issue Mar 28, 2023
Merged
mattansb pushed a commit to easystats/effectsize that referenced this issue Mar 28, 2023
@IndrajeetPatil
Remove commented code in tests (#582)
4c60e28 
cf. easystats/easystats#361
strengejacke added a commit to easystats/report that referenced this issue Mar 28, 2023
@IndrajeetPatil @strengejacke
Uncomment report_sample() tests (#354)
4f8a9d1 
* Uncomment `report_sample()` tests

easystats/easystats#361

* fix: select can be NULL

* proper weighted MAD

* forgot .

---------

Co-authored-by: Daniel <mail@danielluedecke.de>
@etiennebacher etiennebacher mentioned this issue Mar 30, 2023
Merged
@IndrajeetPatil
Copy link
Member Author

bump

IndrajeetPatil added a commit to easystats/bayestestR that referenced this issue Sep 14, 2023
@IndrajeetPatil
Uncomment tests
42d2322 
cf. easystats/easystats#361
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bwiernik bwiernik

@DominiqueMakowski DominiqueMakowski

@IndrajeetPatil IndrajeetPatil

@rempsyc rempsyc

@strengejacke strengejacke

@mattansb mattansb

@etiennebacher etiennebacher

Labels
Code Style 👩‍💻 Core Packages 📦 Discussion and planning about core packages of easystats
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@bwiernik @DominiqueMakowski @IndrajeetPatil @rempsyc @strengejacke @mattansb @etiennebacher