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

Problem: test_sign_offline fails in 0.44 (fix #644) #761

Merged
merged 5 commits into from
May 17, 2022

Conversation

linfeng-crypto
Copy link
Contributor

No description provided.

@tomtau tomtau requested a review from macong-cdc May 4, 2022 07:05
@linfeng-crypto linfeng-crypto changed the title fix sign offline integration test(# 644) fix sign offline integration test(fix #644) May 4, 2022
@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #761 (a7075f2) into master (3fdc974) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #761   +/-   ##
=======================================
  Coverage   17.77%   17.77%           
=======================================
  Files          99       99           
  Lines       10630    10630           
=======================================
  Hits         1890     1890           
  Misses       8300     8300           
  Partials      440      440           
Flag Coverage Δ
integration_tests 17.77% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3fdc974...a7075f2. Read the comment docs.

Copy link
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

it looks all right, just a few questions -- e.g. could chainlibpy use the published PyPI version instead?
+ please follow the contribution guidelines, e.g. the commit message format

chainlibpy = "^1.0.0"
# chainlibpy use version 0.2.3 pystarport
pystarport = "=0.2.3"
chainlibpy = { git = "https://github.com/crypto-org-chain/chainlibpy.git", branch = "master" }
Copy link
Contributor

Choose a reason for hiding this comment

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

can this use the version from PyPI?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to release a new version of chainlibpy.

alice_addr = cluster.address("signer1")
cluster.transfer(alice_addr, offline_addr, "100cro")
wait_for_new_blocks(cluster, 2)
assert cluster.balance(offline_addr) > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

should this assert the amount it was transferred in?

alice_addr = cluster.address("signer1")
cluster.transfer(alice_addr, offline_addr, "100cro")
wait_for_new_blocks(cluster, 2)
assert cluster.balance(offline_addr) > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

should this assert the amount it was transferred in?

@tomtau
Copy link
Contributor

tomtau commented May 10, 2022

good for @macong-cdc to review this, given this is based on #705

@macong-cdc
Copy link
Collaborator

LGTM.
We need to release a new version of chainlibpy for using protobuf to interact with crypto.org chain.

@tomtau
Copy link
Contributor

tomtau commented May 10, 2022

@macong-cdc @linfeng-crypto can 2.2.0 for chainlibpy be released?

chainlibpy = "^1.0.0"
# chainlibpy use version 0.2.3 pystarport
pystarport = "=0.2.3"
chainlibpy = "2.2.0"
Copy link
Contributor

@tomtau tomtau May 12, 2022

Choose a reason for hiding this comment

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

this version doesn't appear to be released on PyPI yet: https://pypi.org/project/chainlibpy/
+ the poetry.lock wasn't updated

Copy link
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

ok by @macong-cdc -- a new version of chainlibpy can be released and the test code can be migrated to it in a separate PR

@tomtau tomtau changed the title fix sign offline integration test(fix #644) Problem: test_sign_offline fails in 0.44 (fix #644) May 17, 2022
@tomtau tomtau merged commit 12edc52 into crypto-org-chain:master May 17, 2022
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.

3 participants