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

395 Fix Failing Unit Tests #2

Merged
merged 2 commits into from
Feb 20, 2024
Merged

395 Fix Failing Unit Tests #2

merged 2 commits into from
Feb 20, 2024

Conversation

NhoxxKienn
Copy link

Description

This PR serves to fix the hyperledger-labs#395 issue of failing unit tests.

App Randomizer Internal Test

Location: channel/test/app_randomizer_internal_test.go

Cause: The issue stems from the lack of Comparator (Equal) for MockAppRandomizer. This leads to the test not recognizing that the default appRandomizer has already been set in the internal test.

Solution: Add an identifier to the MockAppRandomizer to compare the old and new appRandomizer.

Dialer Internal Test

Location: (wire/net/simple/dialer_internal_test.go)

Cause: Like @RmbRT has pointed out, there is a race between the init of wire/simple and backend/sim/wire, which set the RandomAddress and RandomAccount functions to be used in the internal tests.

Solution: Add the initialization of NewRandomAddress and NewRandomAccount to use the implementation from the wire/simple.

Alternative: Remove init() of wire/simple and only use the init of backend/sim/wire. This also results in passing internal tests but might cause issues for packages using wire/simple (need further investigation).

@NhoxxKienn NhoxxKienn force-pushed the 395_fix_failing_unit_tests branch 5 times, most recently from 9a659eb to 99ca7bc Compare February 6, 2024 11:48
@NhoxxKienn NhoxxKienn force-pushed the 395_fix_failing_unit_tests branch from 99ca7bc to b89b2c3 Compare February 6, 2024 11:52
@NhoxxKienn NhoxxKienn self-assigned this Feb 6, 2024
@NhoxxKienn
Copy link
Author

NhoxxKienn commented Feb 16, 2024

One other simple alternative to Dialer Internal Test is to remove the address and account generation functions from wiretest in dialer_internal_test.go. This will force the test to use simple Implementations.

Edit: This alternative solution, however, will not pass the current CI's Unit test because The CI currently still uses Go 1.17 which leads to inconsistent inits.

@NhoxxKienn NhoxxKienn added bug Something isn't working enhancement New feature or request labels Feb 16, 2024
@tinnendo tinnendo merged commit e09ff52 into main Feb 20, 2024
3 of 4 checks passed
@tinnendo tinnendo deleted the 395_fix_failing_unit_tests branch February 20, 2024 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants