chore: lint tests

[faddat]

Description

This PR lints tests, in the same manner we do everything else.

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[faddat]

Hi, @julienrbrt -- going through the ORM tests for the group module was odd

1. Hard to say what they did before, other than satisfy an interface made for testing
2. Currently not too sure what they do either, though I do think that their output involves gas
3. No clear documentation of how they work and they are not tied in with the SDK's normal test framework

---

What I am going to do about it:

1. revert all changes to that folder
2. question the ORM generally speaking because it seems to fall outside of the direction and pattern used in the SDK, and because it relies on a tm-db import, and we are trying to eliminate those

---

Honestly, there's probably more here. I think it might be a good idea to look at pulling orm -- and maybe groups out of 47, before 47 gets 46.6'd.

Also, it's quite late, and we had a power outage today that took down many validators for 20 minutes, so I think that I should re-review with fresh eyes tomorrow.

Relevant discussion: https://blog.jpalardy.com/posts/which-package-name-for-go-tests/

[julienrbrt]

1. revert all changes to that folder

Sure! The complexity may have tricked the linter, I thought if it was showing unused/deadcode it could be safely deleted.

No clear documentation of how they work and they are not tied in with the SDK's normal test framework
question the ORM generally speaking because it seems to fall outside of the direction and pattern used in the SDK, and because it relies on a tm-db import, and we are trying to eliminate those

Yes, ORM severely lack documentation. I don't have enough knowledge with ORM to write about it either. Given that collections are coming, let's see if we'll need documentation at all, or we'll delete it 😅.

[faddat]

fyi: my preference is for this to backport into v0.47.x, 46.x and 45.x, and I know I'll likely need to do it separately for each. Reasoning:

• we likely should lint tests
• generally this has improved testing
• "best practices"

[julienrbrt]

fyi: my preference is for this to backport into v0.47.x, 46.x and 45.x, and I know I'll likely need to do it separately for each. Reasoning:

• we likely should lint tests

• generally this has improved testing

• "best practices"

Yes; but we've decided to be stricter in backports for 0.47; otherwise we need to re-audit. The tests are still working in 0.46 and 0.45 (and they are completely different tests), so no need to retroactively apply this.

This is good that you've done that, and now all the future versions will be lint, so that is still super useful.

[julienrbrt]

lgtm again. just a few things to revert.
pre-approving (but to second reviewer, please do not merge until the above comments are implemented)

[julienrbrt]

given that all fields are private, I don't understand why making this public. can we ignore?

[faddat]

Thanks again for your review. Its surfaced a couple of things that I honestly don't understand and I'm going to run through with the tooling again and make sure that nothing actually caused them to happen. My end goal with this one, is to allow developers working on the SDK to rely on feedback from their editor as heavily as possible.

But for that to work the feedback has to be good.

Fyi, the ones I don't know about are naming the fmt and proto imports.

[julienrbrt]

Putting automerge, lgtm. (otherwise, given the scope, you'll end-up in a conflict hell).

[faddat]

Yeah I totally agree about the conflict how, thanks for that. I'll do a run real quick and make sure that main is now happy.