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

Fixing chaincore and core unit tests #218

Merged
merged 13 commits into from
May 6, 2021
Merged

Conversation

MurashovVen
Copy link
Contributor

Fixed tests:

  1. Removing logs from client_test
  2. Removing logs from node_test
  3. Removing logs from round_test and adding error checking
  4. Removing logs from wallet_test and minor code refactoring
  5. Removing logs from dkg_test and minor code refactoring
  6. Removing logs from codec_test
  7. Tests of util package (merkle_patricia_trie_test, merkle_patricia_trie2_test, merkle_patricia_trie3_test, merkle_trie_test, mpt_nodedb_test):
    removing logs, code refactoring .
  8. Removing old tests from memorystore and adding new
  9. Removing old tests from persistencestore and adding new
  10. Test of encryption package (bls0chain_test, bls0chain_aggregate_test, bls0chain_splittable_test, ed25519_test, encryption_test):
    removing logs

Also added mocks package which includes mocks, used in added tests.

- removing printing out
- add error checking
- removing printing out
- refactoring code
- remove printing out
- refactore code
- refactoring code
- remove printing out
- removing old tests
- adding new tests and increasing test coverage
- removing old tests
- adding new tests and increasing test coverage
@MurashovVen MurashovVen linked an issue Apr 30, 2021 that may be closed by this pull request
…ore_unit_tests

# Conflicts:
#	code/go/0chain.net/go.sum
Copy link
Contributor

@Sriep Sriep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All new unit tests should be checked with the -race option to make sure they are no contributing race issues.

On running the tests with the -race options two tests modified here produced errors.

  • code/go/0chain.net/core/common/codec_test.go Preexisting code. Probably can be handled in Investigate ToMsgpack for race issues. #200, as the whole test needs a rethink.
  • code/go/0chain.net/core/memorystore/store_test.go This is a new test and should not produce confounding race errors. If the race errors here are due to the unit test, they should be fixed. If they are due to the production code; Well done! The description needs updating and an issue raised to fix the race issues.

@MurashovVen
Copy link
Contributor Author

These errors occur because memorystore.DefaultPool isn't protected from concurrent access. We can resolve these race errors by adding global mutex that protects DefaultPool.
Also, trackCollection method runs CollectionTrimmer as a goroutine that needs access to DefaultPool. It makes race test errors and I think we can resolve it by locking mutex when CollectionTrimmer calls GetEntityConnection func.

@Sriep
Copy link
Contributor

Sriep commented May 5, 2021

Added issue #242, to fix memorystore.DefaultPool.

With this, can we close PR #87 Adding core unit tests?

@Sriep Sriep merged commit 4b9439e into master May 6, 2021
@Sriep Sriep deleted the fix/chaincore_and_core_unit_tests branch May 6, 2021 09:50
@MurashovVen
Copy link
Contributor Author

Added issue #242, to fix memorystore.DefaultPool.

With this, can we close PR #87 Adding core unit tests?

Only partially. There are also tests that I would like to decompose into packages and make appropriate PRs:

  • cache
  • common
  • datastore
  • ememorystore
  • encryption
  • logging
  • metric
  • util

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing unit tests.
2 participants