Better code quality

[dmytro-pryvedeniuk]

I am frustrated when I see red/cancelled tests in the main branch, warnings and different code styles.

Proposed solution

Tests

• Disable all red/flaky/cancelled tests, even if they are useful and nice.

• Use zero tolerance to the red tests in a PR to merge. Ask the author to check before merging. Re-run the build. Disable if flaky.

• Create an issue for each disabled test and decide its destiny - fix or remove.

• Make it easier to run the tests. Currently I see the following problems:

  Local setup:

  • Some containers map to the ports that are occupied by the host.
  • Some containers stop before they can be used.
    TestContainers should help - they use dynamic ports and run only when needed.

  GitHub Actions:

  • There is no way to re-run the builds on GitHub. It should be possible to run/re-run manually.
  • The builds are triggered only for main branch (except redis). Lets trigger for all branches.
  • Builds should not fail for a forked repository because of the failed deployment. Lets just silently skip it for the forks. When I merge upstream I usually receive a bunch of "it failed" emails. Less noise - more attention to the real issues.

Warnings

• Currently there is almost 1K of warnings, so introducing real problems is too easy.
• Make the preferred code style clearer. E. g. Async suffix is recommended and there are many warnings about it. If it's "not-our-way" lets disable the warning and make it clear for the contributors.
• editorconfig is desired to define consistent code style.

Visibility

• Use status badges in readme. It can be a selling point itself - number of green tests, code issues etc. See https://github.com/thomhurst/TUnit?tab=readme-ov-file#tunit for example.
• Some static code analysis tool like SonarCube can be useful - actually anything that can provide overview of the current code quality and its evolution.
• Separate channel for contributors with decisions, preferences, diagrams, code styles etc. The current Discord channel looks like more for consumers.

[jeremydmiller]

@dmytro-pryvedeniuk I appreciate the recent help, but please don't make anything remotely like this comment again or I will permanently ban you from this project. Some of this sounds reasonable, some of it sounds like more work than it's worth, and the SonarCube thing is a massive waste of time in my opinion. If you want to help, that's great. If you just want to complain, I can just block you.

The coding standard is explained in the README, so I'm not sure what you're complaining about so much. I don't want to mess with the editorconfig stuff because that's usually a massive waste of time and friction that keeps people from wanting to code in this.

I realize that quite a few tests are flaky. I'm not sure if you've ever worked in a codebase that is absolutely nothing but asynchrous tests, but that's unfortunately common. My approach to that honestly is going to be having the CI run tests more granularly.

[dmytro-pryvedeniuk]

@jeremydmiller Sorry if my proposals look like complaints. Not my intention at all to offend. Quite the opposite. I do like the project and just want to help.

FYI: I have configured SonarCube (OSS license) for a fork - https://sonarcloud.io/project/overview?id=dmytro-pryvedeniuk_wolverine. You can say there are many false issues, like passwords in the sample projects. But after several iterations it can be helpful during code review. Just like anything that helps to find what a human can miss.

Regarding the tests it is a real problem IMO. It's nice that you have so many good tests. But if we accept failures how can we trust the system? You as an owner can't trust a PR. Contributors can't be sure their changes are safe. Bugs can leak. I agree it's hard to make (and even harder to keep) integration tests stable. Better if there is some strategy.

• Each failure is a bug by default.
• Each failure requires investigation.
• False failures should not hide real problems.