Skip to content

added unit tests to sawtooth-core/validator/concurrent/atomic.py#20

Open
mithunshashidhara wants to merge 1 commit intolntdev:mithunfrom
mithunshashidhara:test_concurrent_atomic
Open

added unit tests to sawtooth-core/validator/concurrent/atomic.py#20
mithunshashidhara wants to merge 1 commit intolntdev:mithunfrom
mithunshashidhara:test_concurrent_atomic

Conversation

@mithunshashidhara
Copy link
Copy Markdown
Collaborator

unit tests are added to increase test code coverage.
added tests do not alter existing functionality.

Signed-off-by: mithun shashidhara mithunx.shashidhara@intel.com

unit tests are added to increase test code coverage.
added tests do not alter existing functionality.

Signed-off-by: mithun shashidhara <mithunx.shashidhara@intel.com>
@askmish askmish self-requested a review April 13, 2018 09:06
from test_journal.utils import wait_until

from test_journal import mock_consensus

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please remove unused/redundant imports.

LOGGER = logging.getLogger(__name__)


class Test_ConcurrentMultiMap(unittest.TestCase):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Put some comment what this testclass is about.

if block_validator is not None:
block_validator.stop()


Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Write comments what this class is about.

@askmish
Copy link
Copy Markdown
Collaborator

askmish commented Apr 13, 2018

The commit message subject shouldn't be more than 70 characters.
"added unit tests to sawtooth-core/validator/concurrent/atomic.py"
could be written more meaningfully like for example:
"Add unit tests for atomic.py in validator module"

The commit message body should describe more in detail about what changes happened and stuff:
"unit tests are added to increase test code coverage.
added tests do not alter existing functionality."

Looks very general. Please provide something specific like for example:
"- created a new directory for tests test_concurrent_atomic

  • created a new testcase file
  • Added tests for scenario1, scenario2, etc."

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.

2 participants