Dao test cases #3741

pause125 · 2022-09-18T14:54:37Z

Pull request type

Please check the type of change your PR introduces:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

该 PR 基于 PR Upgrade StarcoinFramework to latest #3718, 并且可以替代 PR Upgrade StarcoinFramework to latest #3718.
移除了一些关于 DAO 的测试用例。这些测试用例可以在 Starcoin-Framework 中测试。
另一些使用了 DAO 的测试用例，应该是具有不同的测试目的。暂时注释了测试代码，并添加了 TODO 提示。如果确实需要保留，后续 stdlib 升级后使用 StarcoinDAO 完成相应测试。如果不再需求，后续则直接删除。

Other information

codecov · 2022-09-18T15:54:40Z

Codecov Report

Merging #3741 (a75d1c5) into master (0a25279) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head a75d1c5 differs from pull request most recent head 3867462. Consider uploading reports for the commit 3867462 to get more accurate results

@@            Coverage Diff             @@
##           master    #3741      +/-   ##
==========================================
- Coverage   55.58%   55.57%   -0.00%     
==========================================
  Files         598      598              
  Lines       64521    64521              
==========================================
- Hits        35858    35854       -4     
- Misses      28663    28667       +4

Flag	Coverage Δ
unittests	`55.57% <100.00%> (-<0.01%)`	⬇️

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

Impacted Files	Coverage Δ
sync/src/tasks/tests.rs	`96.28% <100.00%> (ø)`
...mons/forkable-jellyfish-merkle/src/iterator/mod.rs	`69.79% <0.00%> (-6.11%)`	⬇️
...rc/protocol/generic_proto/upgrade/notifications.rs	`90.89% <0.00%> (-1.34%)`	⬇️
network-p2p/src/protocol/generic_proto/handler.rs	`79.51% <0.00%> (-0.27%)`	⬇️
...etwork-p2p/src/protocol/generic_proto/behaviour.rs	`41.23% <0.00%> (+0.08%)`	⬆️
sync/src/sync.rs	`62.06% <0.00%> (+0.23%)`	⬆️
network-p2p/src/discovery.rs	`73.74% <0.00%> (+0.39%)`	⬆️
commons/service-registry/src/service_ref.rs	`63.45% <0.00%> (+0.69%)`	⬆️
commons/service-registry/src/service_actor.rs	`74.39% <0.00%> (+0.83%)`	⬆️
sync/src/tasks/block_sync_task.rs	`91.52% <0.00%> (+0.90%)`	⬆️
... and 2 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 b01741e...3867462. Read the comment docs.

WGB5445

cmd/starcoin/src/dev/tests.rs test_upgrade_module 这个里面的测试流程也跑不过了，测试依赖以前的 DAO 流程，我觉得可以先屏蔽一下？
看上面的注释可以移到集成测试中
//TODO replace this with integration-test

jiangying000 · 2022-09-19T00:36:23Z

#3733 test_upgrade_module 测试相关

executor/tests/module_upgrade_test.rs

jolestar · 2022-09-19T01:18:53Z

executor/tests/sip_flag_test.rs

-    )?;
-    association_execute_should_success(&net, &chain_state, TransactionPayload::Package(package))?;
-
-    assert!(chain_state.is_activated(sip_10000)?);


感觉 sip 的这个机制的测试要保留下，这个不能在 framework 那边测试。

这个看起来就是提交了一个新的 module ？

对，用 module 作为 feature flag，主要是 chain_state.is_activated(sip_10000) 这个判断机制。可以留个 TODO

pause125 · 2022-09-19T05:54:19Z

有一些测试用例 failed，和 #3723 描述类似。@jiangying000 请问有什么解决办法？

jiangying000 · 2022-09-19T06:23:49Z

有一些测试用例 failed，和 #3723 描述类似。@jiangying000 请问有什么解决办法？

这两个跑了超过60s，超时了

starcoin-sync tasks::tests::test_full_sync_fork_from_genesis
starcoin-sync tasks::tests::test_full_sync_continue

可以把试下函数签名上面的#[stest::test]改成
#[stest::test(timeout = 120)]
或者
#[stest::test(timeout = 180)]

jiangying000 · 2022-09-19T11:40:51Z

cargo test --package starcoin-chain --test test_epoch_switch -- test_modify_on_chain_config_reward_by_dao --exact --nocapture

这个测试没通过

https://github.com/starcoinorg/starcoin/actions/runs/3081866582/jobs/4980893030#step:10:1663

… files.

jiangying000 · 2022-09-19T12:07:22Z

不知道什么原因，有一些测试时间增加了40%，比如：

tasks::tests::test_full_sync_continue 51s -> 71s
tasks::tests::test_full_sync_fork 71s -> 101s

还有一些测试原本配置的超时时间也都超过了，暂时把超时阈值x3，应该能跑过

pause125 · 2022-09-19T12:47:35Z

不知道什么原因，有一些测试时间增加了40%，比如：

tasks::tests::test_full_sync_continue 51s -> 71s tasks::tests::test_full_sync_fork 71s -> 101s

还有一些测试原本配置的超时时间也都超过了，暂时把超时阈值x3，应该能跑过

ok, thx

pause125 · 2022-09-19T14:13:56Z

testsuite integration 测试出现一些 failed, 而且不稳定，每次 fail 的用例不完全一样。这可能是什么原因？和该 PR 的改动有关系吗？ @jolestar

  ✔ Then assert: "{{$.account[-1].ok.ok}} == true"                                              features/cmd.feature:168:5
  ✘ Then cmd: "dev get-coin"                                                                    features/cmd.feature:170:5
———— ! Step failed: ——————————————————————————————————————————————————————————————————— testsuite/tests/steps/cmd.rs:64:21
  future has timed out

pause125 · 2022-09-19T15:01:01Z

testsuite integration 测试出现一些 failed, 而且不稳定，每次 fail 的用例不完全一样。这可能是什么原因？和该 PR 的改动有关系吗？ @jolestar

  ✔ Then assert: "{{$.account[-1].ok.ok}} == true"                                              features/cmd.feature:168:5
  ✘ Then cmd: "dev get-coin"                                                                    features/cmd.feature:170:5
———— ! Step failed: ——————————————————————————————————————————————————————————————————— testsuite/tests/steps/cmd.rs:64:21
  future has timed out

目前定位到 b7957c0 就开始出现这个问题。master 没有问题。 @WGB5445

jolestar · 2022-09-20T00:50:01Z

testsuite integration 测试出现一些 failed, 而且不稳定，每次 fail 的用例不完全一样。这可能是什么原因？和该 PR 的改动有关系吗？ @jolestar
  ✔ Then assert: "{{$.account[-1].ok.ok}} == true"                                              features/cmd.feature:168:5
  ✘ Then cmd: "dev get-coin"                                                                    features/cmd.feature:170:5
———— ! Step failed: ——————————————————————————————————————————————————————————————————— testsuite/tests/steps/cmd.rs:64:21
  future has timed out
目前定位到 b7957c0 就开始出现这个问题。master 没有问题。 @WGB5445

感觉就是超时了。

jolestar · 2022-09-20T01:22:20Z

我本地运行集成测试没有问题，现在不确定那个 feature 超时是在哪里的调用。

pause125 · 2022-09-20T01:26:42Z

我本地运行集成测试没有问题，现在不确定那个 feature 超时是在哪里的调用。

用的这个 commit b7957c0 吗？ CI 挂了，我本地也挂了。用 master 就不会挂。

jolestar · 2022-09-20T01:28:14Z

我本地运行集成测试没有问题，现在不确定那个 feature 超时是在哪里的调用。

用的这个 commit b7957c0 吗？ CI 挂了，我本地也挂了。用 master 就不会挂。

用这个分支的最新 commit，运行多次也没问题

jolestar · 2022-09-20T01:30:32Z

感觉每次 framework 的 module 增加，都会影响测试的时间。需要确认下根本原因。

jolestar · 2022-09-20T02:59:34Z

我先把 timeout 的错误忽略看看 1741d9b

pause125 · 2022-09-20T03:24:27Z

我先把 timeout 的错误忽略看看 1741d9b

和这个 watch timeout 参数有关系吗？可以把 watch timeout 设置长一点？

jolestar · 2022-09-20T03:29:39Z

我先把 timeout 的错误忽略看看 1741d9b

和这个 watch timeout 参数有关系吗？可以把 watch timeout 设置长一点？

已经是 300 秒了，够长了，但看起来是没有等待那么长就返回了。

pause125 · 2022-09-20T03:45:14Z

我先把 timeout 的错误忽略看看 1741d9b

和这个 watch timeout 参数有关系吗？可以把 watch timeout 设置长一点？

已经是 300 秒了，够长了，但看起来是没有等待那么长就返回了。

错误提示是 future time out，可能是 future 函数运行的时候也有 timeout 机制？或者 runtime 触发了 timeout ？

jolestar · 2022-09-20T04:09:45Z

我先把 timeout 的错误忽略看看 1741d9b

和这个 watch timeout 参数有关系吗？可以把 watch timeout 设置长一点？

已经是 300 秒了，够长了，但看起来是没有等待那么长就返回了。

错误提示是 future time out，可能是 future 函数运行的时候也有 timeout 机制？或者 runtime 触发了 timeout ？

这里有用到 future timeout, 现在通过一个 workaround 办法解决了。 @jiangying000 可以帮忙跟踪下

starcoin/rpc/client/src/lib.rs

Lines 188 to 203 in a75d1c5

    
           pub fn watch_txn( 
        
               &self, 
        
               txn_hash: HashValue, 
        
               timeout: Option<Duration>, 
        
           ) -> anyhow::Result<chain_watcher::ThinHeadBlock> { 
        
               let chain_watcher = self.chain_watcher.clone(); 
        
               let f = async move { 
        
                   let r = chain_watcher.send(WatchTxn { txn_hash }).await?; 
        
                   match timeout { 
        
                       Some(t) => async_std::future::timeout(t, r).await??, 
        
                       None => r.await?, 
        
                   } 
        
               }; 
        
               futures::executor::block_on(f) 
        
           }

jiangying000 · 2022-09-20T04:20:12Z

我先把 timeout 的错误忽略看看 1741d9b

和这个 watch timeout 参数有关系吗？可以把 watch timeout 设置长一点？

已经是 300 秒了，够长了，但看起来是没有等待那么长就返回了。

错误提示是 future time out，可能是 future 函数运行的时候也有 timeout 机制？或者 runtime 触发了 timeout ？

这里有用到 future timeout, 现在通过一个 workaround 办法解决了。 @jiangying000 可以帮忙跟踪下

starcoin/rpc/client/src/lib.rs

Lines 188 to 203 in a75d1c5

pub fn watch_txn(

&self,

txn_hash: HashValue,

timeout: Option<Duration>,

) -> anyhow::Result<chain_watcher::ThinHeadBlock> {

let chain_watcher = self.chain_watcher.clone();

let f = async move {

let r = chain_watcher.send(WatchTxn { txn_hash }).await?;

match timeout {

Some(t) => async_std::future::timeout(t, r).await??,

None => r.await?,

}

};

futures::executor::block_on(f)

}

ok

pause125 requested review from jolestar, nanne007 and nkysg as code owners September 18, 2022 14:54

pause125 requested a review from WGB5445 September 18, 2022 14:55

pause125 mentioned this pull request Sep 18, 2022

Daospace support plugin marketplace starcoinorg/starcoin-framework#116

Merged

WGB5445 reviewed Sep 18, 2022

View reviewed changes

WGB5445 mentioned this pull request Sep 18, 2022

Upgrade StarcoinFramework to latest #3718

Closed

7 tasks

jolestar reviewed Sep 19, 2022

View reviewed changes

pause125 force-pushed the dao-test-cases branch from 9f6aec3 to e158e3f Compare September 19, 2022 05:08

pause125 requested a review from templexxx as a code owner September 19, 2022 05:08

jiangying000 mentioned this pull request Sep 19, 2022

[test] larger timeout on full_sync test #3744

Merged

7 tasks

jolestar mentioned this pull request Sep 19, 2022

[test] remove test_upgrade_module #3745

Closed

7 tasks

jiangying000 linked an issue Sep 19, 2022 that may be closed by this pull request

[Dev] replace starcoin-cmd dev::tests::test_upgrade_module with integration-test #3733

Closed

jiangying000 requested a review from yourmoonlight as a code owner September 19, 2022 11:38

WGB5445 and others added 8 commits September 19, 2022 12:01

[stdlib] Add more debug log for stdlib build and regenerate latest mv…

b7957c0

… files.

last Framework

f77b32f

remove some DAO based test cases

0f1373f

fix check

a3bb693

revert sip_flag_test

0bf8981

[test] make timeout 10 min

29bc21b

[test] make all testing timeout * 3

75e2980

update ci config

090952a

remove test cases in test_epoch_switch and error_code_test

c029704

pause125 force-pushed the dao-test-cases branch from 52d40e6 to c029704 Compare September 19, 2022 12:57

[cli] Ignore watch timeout error

1741d9b

[cli] fixup

3867462

jolestar approved these changes Sep 20, 2022

View reviewed changes

jolestar merged commit 365eb36 into starcoinorg:master Sep 20, 2022

jiangying000 mentioned this pull request Sep 21, 2022

[Dev] tracking some test related cases in https://github.com/starcoinorg/starcoin/pull/3741 #3754

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dao test cases #3741

Dao test cases #3741

pause125 commented Sep 18, 2022

codecov bot commented Sep 18, 2022 •

edited

Loading

WGB5445 left a comment •

edited

Loading

jiangying000 commented Sep 19, 2022

jolestar Sep 19, 2022

pause125 Sep 19, 2022

jolestar Sep 19, 2022

pause125 commented Sep 19, 2022

jiangying000 commented Sep 19, 2022 •

edited

Loading

jiangying000 commented Sep 19, 2022 •

edited

Loading

jiangying000 commented Sep 19, 2022 •

edited

Loading

pause125 commented Sep 19, 2022

pause125 commented Sep 19, 2022 •

edited

Loading

pause125 commented Sep 19, 2022

jolestar commented Sep 20, 2022

jolestar commented Sep 20, 2022

pause125 commented Sep 20, 2022

jolestar commented Sep 20, 2022

jolestar commented Sep 20, 2022

jolestar commented Sep 20, 2022

pause125 commented Sep 20, 2022

jolestar commented Sep 20, 2022

pause125 commented Sep 20, 2022

jolestar commented Sep 20, 2022

jiangying000 commented Sep 20, 2022

Dao test cases #3741

Dao test cases #3741

Conversation

pause125 commented Sep 18, 2022

Pull request type

What is the current behavior?

What is the new behavior?

Other information

codecov bot commented Sep 18, 2022 • edited Loading

Codecov Report

WGB5445 left a comment • edited Loading

Choose a reason for hiding this comment

jiangying000 commented Sep 19, 2022

jolestar Sep 19, 2022

Choose a reason for hiding this comment

pause125 Sep 19, 2022

Choose a reason for hiding this comment

jolestar Sep 19, 2022

Choose a reason for hiding this comment

pause125 commented Sep 19, 2022

jiangying000 commented Sep 19, 2022 • edited Loading

jiangying000 commented Sep 19, 2022 • edited Loading

jiangying000 commented Sep 19, 2022 • edited Loading

pause125 commented Sep 19, 2022

pause125 commented Sep 19, 2022 • edited Loading

pause125 commented Sep 19, 2022

jolestar commented Sep 20, 2022

jolestar commented Sep 20, 2022

pause125 commented Sep 20, 2022

jolestar commented Sep 20, 2022

jolestar commented Sep 20, 2022

jolestar commented Sep 20, 2022

pause125 commented Sep 20, 2022

jolestar commented Sep 20, 2022

pause125 commented Sep 20, 2022

jolestar commented Sep 20, 2022

jiangying000 commented Sep 20, 2022

codecov bot commented Sep 18, 2022 •

edited

Loading

WGB5445 left a comment •

edited

Loading

jiangying000 commented Sep 19, 2022 •

edited

Loading

jiangying000 commented Sep 19, 2022 •

edited

Loading

jiangying000 commented Sep 19, 2022 •

edited

Loading

pause125 commented Sep 19, 2022 •

edited

Loading