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.

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

Testing Methods in production code #1487

Open
oxnr opened this issue Jan 17, 2024 · 2 comments
Open

Testing Methods in production code #1487

oxnr opened this issue Jan 17, 2024 · 2 comments
Labels
bb-audit-issue T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Comments

@oxnr
Copy link

oxnr commented Jan 17, 2024

Finding 012 - Testing Methods in production code

ID 012
Finding Testing Methods in production code
Severity 0 - Informational
Description There are testing methods in production code, these testing methods also affect performance because a mutex lock is dependent on a component only used in testing.
Recommendation Move the methods in a testing package, methods in testing packages can be accessed within testing context.
Code References
func (m *Manager) SetDALC(dalc *da.DAClient) {
func (m *Manager) SetLastState(state types.State) {
func (m *Manager) GetStoreHeight() uint64 {
func (m *Manager) GetBlockInCh() chan NewBlockEvent {
func (m *Manager) IsBlockHashSeen(blockHash string) bool {
func (m *Manager) IsDAIncluded(hash types.Hash) bool {
daIncluded map[string]bool
@Manav-Aggarwal Manav-Aggarwal added bb-audit-issue T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. and removed needs-triage labels Jan 18, 2024
@MSevey
Copy link
Contributor

MSevey commented Jan 22, 2024

Test methods can only be accessed within the same package.

golang/go#31135

I believe the advice around defining exports in a test package only really applies to Functions which can live outside a package where an interface or struct is defined.

This methods, are working on unexported fields so they required being defined in the block manager package.

The only way to move them out of production code, would be to move the functionality entirely to mocks within the node package.

@MSevey
Copy link
Contributor

MSevey commented Jan 22, 2024

The alternative would be to define an interface that can then be accessed within the tests. However the interface itself would still be defined within the production code.

Due to the minimal scope of this code, I recommend just closing this issue.

cc @Manav-Aggarwal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bb-audit-issue T:code-hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Projects
Status: No status
Development

No branches or pull requests

3 participants