fix: fix StateMinerSectors tests (#5154)

ChainSafe · Jan 20, 2025 · c899a6d · c899a6d
1 parent 8619510
commit c899a6d
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,8 @@
 - [#4708](https://github.com/ChainSafe/forest/issues/4708) Add support for the
   `Filecoin.EthTraceBlock` RPC method.
 
+- [#5154](https://github.com/ChainSafe/forest/pull/5154) Added support for test criteria overrides in `forest-tool api compare`.
+
 ### Changed
 
 ### Removed

diff --git a/scripts/tests/api_compare/docker-compose.yml b/scripts/tests/api_compare/docker-compose.yml
@@ -210,7 +210,8 @@ services:
         forest-tool api compare $(ls /data/*.car.zst | tail -n 1) \
           --forest $$FOREST_API_INFO \
           --lotus $$LOTUS_API_INFO \
-          --n-tipsets 10 \
+          --n-tipsets 5 \
+          --test-criteria-overrides=valid-and-timeout,timeout-and-timeout \
           --filter-file /data/filter-list \
           --miner-address ${MINER_ADDRESS} \
           --worker-address ${MINER_WORKER_ADDRESS}
@@ -241,7 +242,7 @@ services:
         forest-tool api compare $(ls /data/*.car.zst | tail -n 1) \
           --forest $$FOREST_API_INFO \
           --lotus $$LOTUS_API_INFO \
-          --n-tipsets 10 \
+          --n-tipsets 5 \
           --filter-file /data/filter-list-offline
   post-setup:
     depends_on:

diff --git a/scripts/tests/api_compare/filter-list b/scripts/tests/api_compare/filter-list
@@ -11,5 +11,3 @@
 !Filecoin.EthGetTransactionReceiptLimited
 # TODO: https://github.com/ChainSafe/forest/issues/5006
 !Filecoin.EthGetBlockReceipts
-# TODO: https://github.com/ChainSafe/forest/issues/5100
-!Filecoin.StateMinerSectors
diff --git a/scripts/tests/api_compare/filter-list-offline b/scripts/tests/api_compare/filter-list-offline
@@ -41,5 +41,3 @@
 !Filecoin.EthGetTransactionReceiptLimited
 # TODO: https://github.com/ChainSafe/forest/issues/5006
 !Filecoin.EthGetBlockReceipts
-# TODO: https://github.com/ChainSafe/forest/issues/5100
-!Filecoin.StateMinerSectors
diff --git a/src/tool/subcommands/api_cmd.rs b/src/tool/subcommands/api_cmd.rs
@@ -147,6 +147,10 @@ pub enum ApiCommands {
         /// Specify a directory to which the RPC tests are dumped
         #[arg(long)]
         dump_dir: Option<PathBuf>,
+
+        /// Additional overrides to modify success criteria for tests
+        #[arg(long, value_enum, num_args = 0.., use_value_delimiter = true, value_delimiter = ',', default_values_t = [TestCriteriaOverride::TimeoutAndTimeout])]
+        test_criteria_overrides: Vec<TestCriteriaOverride>,
     },
     GenerateTestSnapshot {
         /// Path to test dumps that are generated by `forest-tool api dump-tests` command
@@ -219,6 +223,7 @@ impl ApiCommands {
                 max_concurrent_requests,
                 create_tests_args,
                 dump_dir,
+                test_criteria_overrides,
             } => {
                 let forest = Arc::new(rpc::Client::from_url(forest));
                 let lotus = Arc::new(rpc::Client::from_url(lotus));
@@ -237,6 +242,7 @@ impl ApiCommands {
                         run_ignored,
                         fail_fast,
                         dump_dir.clone(),
+                        &test_criteria_overrides,
                     )
                     .await?;
                 }
@@ -367,6 +373,14 @@ pub struct CreateTestsArgs {
     snapshot_files: Vec<PathBuf>,
 }
 
+#[derive(Debug, Copy, Clone, PartialEq, ValueEnum)]
+pub enum TestCriteriaOverride {
+    /// Test pass when first endpoint returns a valid result and the second one timeout
+    ValidAndTimeout,
+    /// Test pass when both endpoints timeout
+    TimeoutAndTimeout,
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 pub struct Dialogue {
     method: String,
@@ -661,15 +675,6 @@ fn common_tests() -> Vec<RpcTest> {
     ]
 }
 
-fn beacon_tests() -> Vec<RpcTest> {
-    // TODO(forest): https://github.com/ChainSafe/forest/issues/4718
-    // Blocked by Lotus.
-    //vec![RpcTest::identity(
-    //    BeaconGetEntry::request((10101,)).unwrap(),
-    //)]
-    vec![]
-}
-
 fn chain_tests() -> Vec<RpcTest> {
     vec![
         RpcTest::basic(ChainHead::request(()).unwrap()),
@@ -808,10 +813,9 @@ fn node_tests() -> Vec<RpcTest> {
 fn state_tests() -> Vec<RpcTest> {
     // TODO(forest): https://github.com/ChainSafe/forest/issues/4718
     // Blocked by Lotus.
-    //vec![
-    //    RpcTest::identity(StateGetBeaconEntry::request((0.into(),)).unwrap()),
-    //    RpcTest::identity(StateGetBeaconEntry::request((1.into(),)).unwrap()),
-    //]
+    //vec![RpcTest::identity(
+    //    BeaconGetEntry::request((10101,)).unwrap(),
+    //)]
     vec![]
 }
 
@@ -1839,7 +1843,6 @@ fn create_tests(
     let mut tests = vec![];
     tests.extend(auth_tests()?);
     tests.extend(common_tests());
-    tests.extend(beacon_tests());
     tests.extend(chain_tests());
     tests.extend(mpool_tests());
     tests.extend(net_tests());
@@ -1888,6 +1891,7 @@ async fn run_tests(
     run_ignored: RunIgnored,
     fail_fast: bool,
     dump_dir: Option<PathBuf>,
+    test_criteria_overrides: &[TestCriteriaOverride],
 ) -> anyhow::Result<()> {
     let forest = Into::<Arc<rpc::Client>>::into(forest);
     let lotus = Into::<Arc<rpc::Client>>::into(lotus);
@@ -1955,8 +1959,13 @@ async fn run_tests(
         let forest_status = test_result.forest_status;
         let lotus_status = test_result.lotus_status;
         let success = match (&forest_status, &lotus_status) {
-            (TestSummary::Valid, TestSummary::Valid)
-            | (TestSummary::Timeout, TestSummary::Timeout) => true,
+            (TestSummary::Valid, TestSummary::Valid) => true,
+            (TestSummary::Valid, TestSummary::Timeout) => {
+                test_criteria_overrides.contains(&TestCriteriaOverride::ValidAndTimeout)
+            }
+            (TestSummary::Timeout, TestSummary::Timeout) => {
+                test_criteria_overrides.contains(&TestCriteriaOverride::TimeoutAndTimeout)
+            }
             (TestSummary::Rejected(ref reason_forest), TestSummary::Rejected(ref reason_lotus)) => {
                 match test.policy_on_rejected {
                     PolicyOnRejected::Pass => true,