From 260705280faa7e1f41b4442c68da26e734c2836a Mon Sep 17 00:00:00 2001 From: "Nathan.fooo" <86001920+appflowy@users.noreply.github.com> Date: Mon, 19 Dec 2022 10:47:40 +0800 Subject: [PATCH] fix: remove unused steps (#1580) * chore: remove unused code * chore: update pin-project version to remove rust lint warnings * chore: fix potential test failed Co-authored-by: nathan --- .github/workflows/rust_coverage.yml | 53 ++++++++----------- frontend/rust-lib/Cargo.lock | 8 +-- frontend/rust-lib/flowy-folder/Cargo.toml | 2 +- .../src/services/filter/controller.rs | 19 +++---- .../tests/grid/filter_test/script.rs | 48 ++++++++++------- .../grid/filter_test/text_filter_test.rs | 10 ++-- frontend/rust-lib/flowy-sdk/src/lib.rs | 1 + frontend/rust-lib/flowy-task/src/scheduler.rs | 4 +- frontend/rust-lib/lib-dispatch/Cargo.toml | 2 +- frontend/scripts/makefile/tests.toml | 34 +++--------- shared-lib/Cargo.lock | 8 +-- shared-lib/grid-rev-model/src/grid_rev.rs | 1 - shared-lib/lib-infra/Cargo.toml | 2 +- shared-lib/lib-ws/Cargo.toml | 2 +- 14 files changed, 89 insertions(+), 105 deletions(-) diff --git a/.github/workflows/rust_coverage.yml b/.github/workflows/rust_coverage.yml index 512796cb93..cad1af4a6c 100644 --- a/.github/workflows/rust_coverage.yml +++ b/.github/workflows/rust_coverage.yml @@ -1,4 +1,4 @@ -name: Rust coverage tests +name: Rust code coverage on: push: @@ -9,25 +9,16 @@ on: - "frontend/rust-lib/**" - "shared-lib/**" - pull_request: - branches: - - "main" - - "release/*" - paths: - - "frontend/rust-lib/**" - - "shared-lib/**" - env: CARGO_TERM_COLOR: always - jobs: - test-coverage: + tests: runs-on: ubuntu-latest steps: - name: Checkout uses: actions/checkout@v2 - + - id: rust_toolchain uses: actions-rs/toolchain@v1 with: @@ -35,39 +26,41 @@ jobs: - name: Cache Cargo uses: actions/cache@v2 - with: + with: path: | ~/.cargo key: ${{ runner.os }}-cargo-${{ steps.rust_toolchain.outputs.rustc_hash }}-${{ hashFiles('./frontend/rust-lib/Cargo.toml') }} - name: Cache Rust uses: actions/cache@v2 - with: + with: path: | frontend/rust-lib/target shared-lib/target - key: ${{ runner.os }}-rust-rust-lib-share-lib-${{ steps.rust_toolchain.outputs.rustc_hash }}-${{ hashFiles('./frontend/rust-lib/Cargo.toml') }} + key: ${{ runner.os }}-rust-rust-lib-share-lib-${{ steps.rust_toolchain.outputs.rustc_hash }}-${{ hashFiles('./frontend/rust-lib/Cargo.toml') }} - - name: Setup Environment - run: | - if [ "$RUNNER_OS" == "Linux" ]; then - sudo wget -qO /etc/apt/trusted.gpg.d/dart_linux_signing_key.asc https://dl-ssl.google.com/linux/linux_signing_key.pub - sudo wget -qO /etc/apt/sources.list.d/dart_stable.list https://storage.googleapis.com/download.dartlang.org/linux/debian/dart_stable.list - sudo apt-get update - sudo apt-get install -y dart curl build-essential libsqlite3-dev libssl-dev clang cmake ninja-build pkg-config libgtk-3-dev - sudo apt-get install keybinder-3.0 - elif [ "$RUNNER_OS" == "macOS" ]; then - echo 'do nothing' - fi - shell: bash - - name: Install cargo-make, grcov and llvm-tools-preview + - name: Install cargo-make working-directory: frontend run: | cargo install cargo-make + + - name: Setup environment - Flutter + uses: subosito/flutter-action@v2 + with: + channel: 'stable' + flutter-version: '3.0.5' + + - name: Install code-coverage tools + working-directory: frontend + run: | + sudo wget -qO /etc/apt/trusted.gpg.d/dart_linux_signing_key.asc https://dl-ssl.google.com/linux/linux_signing_key.pub + sudo apt-get update + sudo apt-get install -y build-essential libsqlite3-dev libssl-dev clang cmake ninja-build pkg-config libgtk-3-dev + sudo apt-get install keybinder-3.0 cargo install grcov rustup component add llvm-tools-preview - - name: Run Coverage tests and generate LCOV report + - name: Run tests working-directory: frontend - run: cargo make get_ci_test_coverage + run: cargo make rust_unit_test_with_coverage diff --git a/frontend/rust-lib/Cargo.lock b/frontend/rust-lib/Cargo.lock index ac0f61be73..00ab486dc2 100644 --- a/frontend/rust-lib/Cargo.lock +++ b/frontend/rust-lib/Cargo.lock @@ -2380,18 +2380,18 @@ dependencies = [ [[package]] name = "pin-project" -version = "1.0.10" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "58ad3879ad3baf4e44784bc6a718a8698867bb991f8ce24d1bcbe2cfb4c3a75e" +checksum = "ad29a609b6bcd67fee905812e544992d216af9d755757c05ed2d0e15a74c6ecc" dependencies = [ "pin-project-internal", ] [[package]] name = "pin-project-internal" -version = "1.0.10" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "744b6f092ba29c3650faf274db506afd39944f48420f6c86b17cfe0ee1cb36bb" +checksum = "069bdb1e05adc7a8990dce9cc75370895fbe4e3d58b9b73bf1aee56359344a55" dependencies = [ "proc-macro2", "quote", diff --git a/frontend/rust-lib/flowy-folder/Cargo.toml b/frontend/rust-lib/flowy-folder/Cargo.toml index 3c9a02f09a..ebd4ca4dd4 100644 --- a/frontend/rust-lib/flowy-folder/Cargo.toml +++ b/frontend/rust-lib/flowy-folder/Cargo.toml @@ -26,7 +26,7 @@ log = "0.4.14" diesel = {version = "1.4.8", features = ["sqlite"]} diesel_derives = {version = "1.4.1", features = ["sqlite"]} futures = "0.3.15" -pin-project = "1.0.0" +pin-project = "1.0" strum = "0.21" strum_macros = "0.21" tokio = { version = "1", features = ["rt"] } diff --git a/frontend/rust-lib/flowy-grid/src/services/filter/controller.rs b/frontend/rust-lib/flowy-grid/src/services/filter/controller.rs index 028de3a06a..c21cd8b0c9 100644 --- a/frontend/rust-lib/flowy-grid/src/services/filter/controller.rs +++ b/frontend/rust-lib/flowy-grid/src/services/filter/controller.rs @@ -68,14 +68,9 @@ impl FilterController { } #[tracing::instrument(name = "schedule_filter_task", level = "trace", skip(self))] - async fn gen_task(&mut self, task_type: FilterEvent) { + async fn gen_task(&mut self, task_type: FilterEvent, qos: QualityOfService) { let task_id = self.task_scheduler.read().await.next_task_id(); - let task = Task::new( - &self.handler_id, - task_id, - TaskContent::Text(task_type.to_string()), - QualityOfService::UserInteractive, - ); + let task = Task::new(&self.handler_id, task_id, TaskContent::Text(task_type.to_string()), qos); self.task_scheduler.write().await.add_task(task); } @@ -187,7 +182,11 @@ impl FilterController { } pub async fn did_receive_row_changed(&mut self, row_id: &str) { - self.gen_task(FilterEvent::RowDidChanged(row_id.to_string())).await + self.gen_task( + FilterEvent::RowDidChanged(row_id.to_string()), + QualityOfService::UserInteractive, + ) + .await } #[tracing::instrument(level = "trace", skip(self))] @@ -237,7 +236,9 @@ impl FilterController { self.filter_map.remove(filter_type); } - let _ = self.gen_task(FilterEvent::FilterDidChanged).await; + let _ = self + .gen_task(FilterEvent::FilterDidChanged, QualityOfService::Background) + .await; tracing::trace!("{:?}", notification); notification } diff --git a/frontend/rust-lib/flowy-grid/tests/grid/filter_test/script.rs b/frontend/rust-lib/flowy-grid/tests/grid/filter_test/script.rs index 30d5187faa..2461d5e6f0 100644 --- a/frontend/rust-lib/flowy-grid/tests/grid/filter_test/script.rs +++ b/frontend/rust-lib/flowy-grid/tests/grid/filter_test/script.rs @@ -6,6 +6,7 @@ use std::time::Duration; use bytes::Bytes; use futures::TryFutureExt; +use tokio::sync::broadcast::Receiver; use flowy_grid::entities::{AlterFilterParams, AlterFilterPayloadPB, DeleteFilterParams, GridLayout, GridSettingChangesetParams, GridSettingPB, RowPB, TextFilterConditionPB, FieldType, NumberFilterConditionPB, CheckboxFilterConditionPB, DateFilterConditionPB, DateFilterContentPB, SelectOptionConditionPB, TextFilterPB, NumberFilterPB, CheckboxFilterPB, DateFilterPB, SelectOptionFilterPB, CellChangesetPB, FilterPB, ChecklistFilterConditionPB, ChecklistFilterPB}; use flowy_grid::services::field::{SelectOptionCellChangeset, SelectOptionIds}; use flowy_grid::services::setting::GridSettingChangesetBuilder; @@ -89,13 +90,15 @@ pub enum FilterScript { pub struct GridFilterTest { inner: GridEditorTest, + recv: Option>, } impl GridFilterTest { pub async fn new() -> Self { let editor_test = GridEditorTest::new_table().await; Self { - inner: editor_test + inner: editor_test, + recv: None, } } @@ -116,17 +119,19 @@ impl GridFilterTest { pub async fn run_script(&mut self, script: FilterScript) { match script { FilterScript::UpdateTextCell { row_index, text} => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); self.update_text_cell(row_index, &text).await; } - FilterScript::UpdateSingleSelectCell { row_index, option_id} => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); self.update_single_select_cell(row_index, &option_id).await; } FilterScript::InsertFilter { payload } => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); self.insert_filter(payload).await; } FilterScript::CreateTextFilter { condition, content} => { - + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); let field_rev = self.get_first_field_rev(FieldType::RichText); let text_filter= TextFilterPB { condition, @@ -139,6 +144,7 @@ impl GridFilterTest { self.insert_filter(payload).await; } FilterScript::UpdateTextFilter { filter, condition, content} => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); let params = AlterFilterParams { view_id: self.view_id(), field_id: filter.field_id, @@ -150,6 +156,7 @@ impl GridFilterTest { self.editor.create_or_update_filter(params).await.unwrap(); } FilterScript::CreateNumberFilter {condition, content} => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); let field_rev = self.get_first_field_rev(FieldType::Number); let number_filter = NumberFilterPB { condition, @@ -162,6 +169,7 @@ impl GridFilterTest { self.insert_filter(payload).await; } FilterScript::CreateCheckboxFilter {condition} => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); let field_rev = self.get_first_field_rev(FieldType::Checkbox); let checkbox_filter = CheckboxFilterPB { condition @@ -171,6 +179,7 @@ impl GridFilterTest { self.insert_filter(payload).await; } FilterScript::CreateDateFilter { condition, start, end, timestamp} => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); let field_rev = self.get_first_field_rev(FieldType::DateTime); let date_filter = DateFilterPB { condition, @@ -184,6 +193,7 @@ impl GridFilterTest { self.insert_filter(payload).await; } FilterScript::CreateMultiSelectFilter { condition, option_ids} => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); let field_rev = self.get_first_field_rev(FieldType::MultiSelect); let filter = SelectOptionFilterPB { condition, option_ids }; let payload = @@ -191,6 +201,7 @@ impl GridFilterTest { self.insert_filter(payload).await; } FilterScript::CreateSingleSelectFilter { condition, option_ids} => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); let field_rev = self.get_first_field_rev(FieldType::SingleSelect); let filter = SelectOptionFilterPB { condition, option_ids }; let payload = @@ -198,6 +209,7 @@ impl GridFilterTest { self.insert_filter(payload).await; } FilterScript::CreateChecklistFilter { condition} => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); let field_rev = self.get_first_field_rev(FieldType::Checklist); // let type_option = self.get_checklist_type_option(&field_rev.id); let filter = ChecklistFilterPB { condition }; @@ -216,6 +228,7 @@ impl GridFilterTest { } FilterScript::DeleteFilter { filter_id, filter_type } => { + self.recv = Some(self.editor.subscribe_view_changed(&self.view_id()).await.unwrap()); let params = DeleteFilterParams { view_id: self.view_id(),filter_type, filter_id }; let _ = self.editor.delete_filter(params).await.unwrap(); } @@ -224,24 +237,21 @@ impl GridFilterTest { assert_eq!(expected_setting, setting); } FilterScript::AssertFilterChanged { visible_row_len, hide_row_len} => { - let editor = self.editor.clone(); - let view_id = self.view_id(); - let mut receiver = - tokio::spawn(async move { - editor.subscribe_view_changed(&view_id).await.unwrap() - }).await.unwrap(); - match tokio::time::timeout(Duration::from_secs(2), receiver.recv()).await { - Ok(changed) => { - // - match changed.unwrap() { GridViewChanged::DidReceiveFilterResult(changed) => { - assert_eq!(changed.visible_rows.len(), visible_row_len, "visible rows not match"); - assert_eq!(changed.invisible_rows.len(), hide_row_len, "invisible rows not match"); - } } - }, - Err(e) => { - panic!("Process task timeout: {:?}", e); + if let Some(mut receiver) = self.recv.take() { + match tokio::time::timeout(Duration::from_secs(2), receiver.recv()).await { + Ok(changed) => { + // + match changed.unwrap() { GridViewChanged::DidReceiveFilterResult(changed) => { + assert_eq!(changed.visible_rows.len(), visible_row_len, "visible rows not match"); + assert_eq!(changed.invisible_rows.len(), hide_row_len, "invisible rows not match"); + } } + }, + Err(e) => { + panic!("Process task timeout: {:?}", e); + } } } + } FilterScript::AssertNumberOfVisibleRows { expected } => { let grid = self.editor.get_grid(&self.view_id()).await.unwrap(); diff --git a/frontend/rust-lib/flowy-grid/tests/grid/filter_test/text_filter_test.rs b/frontend/rust-lib/flowy-grid/tests/grid/filter_test/text_filter_test.rs index 7e07b3cd01..916f454a84 100644 --- a/frontend/rust-lib/flowy-grid/tests/grid/filter_test/text_filter_test.rs +++ b/frontend/rust-lib/flowy-grid/tests/grid/filter_test/text_filter_test.rs @@ -30,6 +30,7 @@ async fn grid_filter_text_is_not_empty_test() { content: "".to_string(), }, AssertFilterCount { count: 1 }, + // There is only one row in the test data that its text is empty AssertFilterChanged { visible_row_len: 0, hide_row_len: 1, @@ -44,10 +45,11 @@ async fn grid_filter_text_is_not_empty_test() { filter_id: filter.id, filter_type: FilterType::from(&field_rev), }, - // AssertFilterChanged { - // visible_row_len: 1, - // hide_row_len: 0, - // }, + AssertFilterCount { count: 0 }, + AssertFilterChanged { + visible_row_len: 1, + hide_row_len: 0, + }, ]) .await; } diff --git a/frontend/rust-lib/flowy-sdk/src/lib.rs b/frontend/rust-lib/flowy-sdk/src/lib.rs index 35d898e57a..ecaa354c1b 100644 --- a/frontend/rust-lib/flowy-sdk/src/lib.rs +++ b/frontend/rust-lib/flowy-sdk/src/lib.rs @@ -89,6 +89,7 @@ fn crate_log_filter(level: String) -> String { filters.push(format!("lib_infra={}", level)); filters.push(format!("flowy_sync={}", level)); filters.push(format!("flowy_revision={}", level)); + filters.push(format!("flowy_task={}", level)); // filters.push(format!("lib_dispatch={}", level)); filters.push(format!("dart_ffi={}", "info")); diff --git a/frontend/rust-lib/flowy-task/src/scheduler.rs b/frontend/rust-lib/flowy-task/src/scheduler.rs index d06d5a54d3..b0d9998210 100644 --- a/frontend/rust-lib/flowy-task/src/scheduler.rs +++ b/frontend/rust-lib/flowy-task/src/scheduler.rs @@ -71,12 +71,12 @@ impl TaskDispatcher { Ok(result) => match result { Ok(_) => task.set_state(TaskState::Done), Err(e) => { - tracing::error!("Process task failed: {:?}", e); + tracing::error!("Process {} task failed: {:?}", handler.handler_id(), e); task.set_state(TaskState::Failure); } }, Err(e) => { - tracing::error!("Process task timeout: {:?}", e); + tracing::error!("Process {} task timeout: {:?}", handler.handler_id(), e); task.set_state(TaskState::Timeout); } } diff --git a/frontend/rust-lib/lib-dispatch/Cargo.toml b/frontend/rust-lib/lib-dispatch/Cargo.toml index d458c6c41b..68afc4960a 100644 --- a/frontend/rust-lib/lib-dispatch/Cargo.toml +++ b/frontend/rust-lib/lib-dispatch/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -pin-project = "1.0.0" +pin-project = "1.0" futures-core = { version = "0.3", default-features = false } paste = "1" futures-channel = "0.3.15" diff --git a/frontend/scripts/makefile/tests.toml b/frontend/scripts/makefile/tests.toml index d8a3bd7089..0140887293 100644 --- a/frontend/scripts/makefile/tests.toml +++ b/frontend/scripts/makefile/tests.toml @@ -67,19 +67,12 @@ script_runner = "@shell" script = [ """ echo --- Running coverage tests --- - - # Install Protobuf compiler - cargo make install_protobuf_compiler - export PATH="$PATH":"$HOME/.pub-cache/bin" - export PATH="$PATH":"$HOME/.cargo/bin/" - cd rust-lib/ CARGO_INCREMENTAL=0 \ RUSTFLAGS='-C instrument-coverage' \ LLVM_PROFILE_FILE='prof-%p-%m.profraw' \ cargo test --no-default-features --features="sync" - """ ] @@ -89,18 +82,12 @@ script_runner = "@shell" script = [ """ echo --- Running coverage tests --- - - # Install Protobuf compiler - cargo make install_protobuf_compiler - export PATH="$PATH":"$HOME/.pub-cache/bin" - export PATH="$PATH":"$HOME/.cargo/bin/" - cd ../shared-lib CARGO_INCREMENTAL=0 \ RUSTFLAGS='-C instrument-coverage' \ LLVM_PROFILE_FILE='prof-%p-%m.profraw' \ - cargo test --no-default-features + cargo test --no-default-features """ ] @@ -110,7 +97,7 @@ description = "Get `grcov` HTML report for test coverage for rust-lib" script_runner = "@shell" script = [ """ - echo --- Getting 'grcov' results for 'rust-lib' --- + echo --- Getting 'grcov' results for 'rust-lib' --- cd rust-lib/ grcov . \ @@ -131,7 +118,7 @@ description = "Get `grcov` HTML report for test coverage shared-lib" script_runner = "@shell" script = [ """ - echo --- Getting 'grcov' results 'shared-lib' --- + echo --- Getting 'grcov' results 'shared-lib' --- cd ../shared-lib grcov . \ @@ -205,22 +192,13 @@ run_task = { name = [ "get_rustlib_lcov_report" ], parallel = true } -[tasks.get_ci_test_coverage] -description = "Get LCOV coverage reports for CI" +[tasks.rust_unit_test_with_coverage] +description = "Run rust unit test with code coverage" run_task = { name = [ "check_grcov", + 'appflowy-deps-tools', "run_rustlib_coverage_tests", "run_sharedlib_coverage_tests", "get_lcov_report", "clean_profraw_files" ]} - -[tasks.get_test_coverage] -description = "Get human readable test coverage reports" -run_task = { name = [ - "check_grcov", - "run_rustlib_coverage_tests", - "run_sharedlib_coverage_tests", - "get_grcov_report", - "clean_profraw_files" - ]} diff --git a/shared-lib/Cargo.lock b/shared-lib/Cargo.lock index bae8b51b22..125bb8c48c 100644 --- a/shared-lib/Cargo.lock +++ b/shared-lib/Cargo.lock @@ -1221,18 +1221,18 @@ dependencies = [ [[package]] name = "pin-project" -version = "1.0.8" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "576bc800220cc65dac09e99e97b08b358cfab6e17078de8dc5fee223bd2d0c08" +checksum = "ad29a609b6bcd67fee905812e544992d216af9d755757c05ed2d0e15a74c6ecc" dependencies = [ "pin-project-internal", ] [[package]] name = "pin-project-internal" -version = "1.0.8" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e8fe8163d14ce7f0cdac2e040116f22eac817edabff0be91e8aff7e9accf389" +checksum = "069bdb1e05adc7a8990dce9cc75370895fbe4e3d58b9b73bf1aee56359344a55" dependencies = [ "proc-macro2", "quote", diff --git a/shared-lib/grid-rev-model/src/grid_rev.rs b/shared-lib/grid-rev-model/src/grid_rev.rs index 8074a6532f..5bf7b8c4ab 100644 --- a/shared-lib/grid-rev-model/src/grid_rev.rs +++ b/shared-lib/grid-rev-model/src/grid_rev.rs @@ -3,7 +3,6 @@ use bytes::Bytes; use indexmap::IndexMap; use nanoid::nanoid; use serde::{Deserialize, Serialize}; -use std::str::FromStr; use std::sync::Arc; pub fn gen_grid_id() -> String { diff --git a/shared-lib/lib-infra/Cargo.toml b/shared-lib/lib-infra/Cargo.toml index b36beb1847..8ee89b1b44 100644 --- a/shared-lib/lib-infra/Cargo.toml +++ b/shared-lib/lib-infra/Cargo.toml @@ -8,7 +8,7 @@ edition = "2018" [dependencies] chrono = "0.4.19" bytes = { version = "1.0" } -pin-project = "1.0" +pin-project = "1.0.12" futures-core = { version = "0.3" } tokio = { version = "1.0", features = ["time", "rt"] } rand = "0.8.5" diff --git a/shared-lib/lib-ws/Cargo.toml b/shared-lib/lib-ws/Cargo.toml index 0261cab886..b57c98d836 100644 --- a/shared-lib/lib-ws/Cargo.toml +++ b/shared-lib/lib-ws/Cargo.toml @@ -15,7 +15,7 @@ futures-channel = "0.3.17" tokio = {version = "1", features = ["full"]} futures = "0.3.17" bytes = "1.0" -pin-project = "1.0.0" +pin-project = "1.0" futures-core = { version = "0.3", default-features = false } paste = "1" url = "2.2.2"