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

hotfix: Shapefile 用のフィールド名辞書を更新する #545

Merged
merged 1 commit into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/src-tauri/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,11 @@ fn run_conversion(
.unwrap();

// Wait for the pipeline to finish
handle.join();
if let Err(msg) = handle.join() {
return Err(Error::ConversionFailed(format!(
"Pipeline thread panicked: {msg}"
)));
}
Comment on lines +263 to +267
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

run_conversion 関数の handle.join() エラーハンドリングがテストでカバーされていないようです。この新しいエラーハンドリングロジックをカバーするために、特定のテストケースを追加することをお勧めします。

  • run_conversion 関数の handle.join() エラーハンドリングをカバーするテストケースを追加してください。
Analysis chain

run_conversion 関数に追加された handle.join() のエラーハンドリングにより、スレッドの結合中に発生する可能性のあるエラーを適切に処理できるようになりました。この変更は、エラーハンドリングを強化するというプルリクエストの目的に合致しています。

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify if the new line is covered by tests.

# Test: Search for the test coverage of the line. Expect: Coverage report showing the line.
rg --type rust $'if let Err(msg) = handle.join()' app/src-tauri/src/main.rs

Length of output: 76



Script:

#!/bin/bash
# Description: Search for test functions that might cover the `handle.join()` call in `run_conversion`.

# Find all test functions in the codebase.
rg --type rust '#\[test\]' -A 1

# Search for invocations of `run_conversion` in the test files.
rg 'run_conversion' --type rust

Length of output: 25084



Script:

#!/bin/bash
# Description: Search for functions that call `run_conversion` and check their test coverage.

# Find all functions that call `run_conversion`.
rg 'run_conversion' --type rust -A 3

# Check if these functions are covered by tests.
rg --type rust '#\[test\]' -A 1

Length of output: 25426


// Return error if an error occurred in the pipeline
if let Some(err) = first_error {
Expand Down
5 changes: 4 additions & 1 deletion nusamai/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,10 @@
});

// wait for the pipeline to finish
handle.join();
if let Err(msg) = handle.join() {
log::error!("Pipeline thread panicked: {:?}", msg);

Check warning on line 287 in nusamai/src/main.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/main.rs#L287

Added line #L287 was not covered by tests
}

Comment on lines +286 to +289
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

main 関数に追加された handle.join() のエラーハンドリングにより、スレッドの結合中に発生する可能性のあるエラーを適切に処理できるようになりました。この変更は、エラーハンドリングを強化するというプルリクエストの目的に合致しています。ただし、現在のテストではこのエラーハンドリングがカバーされていないようです。新しいエラーハンドリング行をカバーするテストを追加することをお勧めします。

  • nusamai/tests/sink.rs: handle.join().unwrap();
  • nusamai/tests/pipeline.rs: handle.join().unwrap();
Analysis chain

main 関数に追加された handle.join() のエラーハンドリングにより、スレッドの結合中に発生する可能性のあるエラーを適切に処理できるようになりました。この変更は、エラーハンドリングを強化するというプルリクエストの目的に合致しています。

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify if the new line is covered by tests.

# Test: Search for the test coverage of the line. Expect: Coverage report showing the line.
rg --type rust $'if let Err(msg) = handle.join()' nusamai/src/main.rs

Length of output: 70



Script:

#!/bin/bash
# Description: Search for test functions related to thread operations and joining threads.

# Find all Rust test functions in the codebase.
rg --type rust --pattern '#\[test\]' --context 10

# Specifically look for thread operations and joining threads in test files.
rg --type rust 'thread::spawn' --context 10
rg --type rust 'handle.join()' --context 10

Length of output: 6594



Script:

#!/bin/bash
# Description: Search for test functions that include `handle.join()` and handle errors.

# Find all Rust test functions that include `handle.join()`.
rg --type rust 'handle.join()' --context 10 | rg --type rust '#\[test\]' --context 10

Length of output: 1122

if canceller.lock().unwrap().is_canceled() {
log::info!("Pipeline canceled");
}
Expand Down
27 changes: 20 additions & 7 deletions nusamai/src/pipeline/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,29 @@

impl PipelineHandle {
// Wait for the pipeline to terminate
pub fn join(self) {
if self.source_thread_handle.join().is_err() {
log::error!("Source thread panicked");
pub fn join(self) -> Result<(), String> {
fn report_error(stage: &str, err: Box<dyn std::any::Any + Send>) -> String {
let msg = if let Some(message) = err.downcast_ref::<&str>() {
format!("{stage} thread panicked with message: {}", message)
} else if let Some(string) = err.downcast_ref::<String>() {
format!("{stage} thread panicked with message: {}", string)

Check warning on line 118 in nusamai/src/pipeline/runner.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/pipeline/runner.rs#L114-L118

Added lines #L114 - L118 were not covered by tests
} else {
format!("{stage} thread panicked with an unknown type.")

Check warning on line 120 in nusamai/src/pipeline/runner.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/pipeline/runner.rs#L120

Added line #L120 was not covered by tests
};
log::error!("{}", msg);
msg.to_string()

Check warning on line 123 in nusamai/src/pipeline/runner.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/pipeline/runner.rs#L122-L123

Added lines #L122 - L123 were not covered by tests
}
if self.transformer_thread_handle.join().is_err() {
log::error!("Transformer thread panicked");

if let Err(err) = self.source_thread_handle.join() {
return Err(report_error("Source", err));

Check warning on line 127 in nusamai/src/pipeline/runner.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/pipeline/runner.rs#L127

Added line #L127 was not covered by tests
}
if let Err(err) = self.transformer_thread_handle.join() {
return Err(report_error("Transformer", err));

Check warning on line 130 in nusamai/src/pipeline/runner.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/pipeline/runner.rs#L130

Added line #L130 was not covered by tests
}
if self.sink_thread_handle.join().is_err() {
log::error!("Sink thread panicked");
if let Err(err) = self.sink_thread_handle.join() {
return Err(report_error("Sink", err));

Check warning on line 133 in nusamai/src/pipeline/runner.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/pipeline/runner.rs#L133

Added line #L133 was not covered by tests
}
Ok(())
Comment on lines +113 to +135
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

join 関数の新しいエラーハンドリングロジックがテストでカバーされているか確認するために、テストファイル nusamai/tests/sink.rsnusamai/tests/pipeline.rs を調査しました。これらのテストは join メソッドを呼び出していますが、新しいエラーハンドリングロジックを明示的にテストしているわけではありません。

以下の点を確認する必要があります:

  • join メソッドのエラーハンドリングロジックが適切にテストされているか。

このためには、エラーハンドリングロジックをテストするための新しいテストケースを追加することをお勧めします。

  • join メソッドのエラーハンドリングロジックをテストする新しいテストケースを追加してください。
Analysis chain

join 関数に追加されたエラーハンドリングにより、スレッドの結合中に発生する可能性のあるエラーを特定し、適切にログを出力することができるようになりました。この変更は、エラーハンドリングを強化するというプルリクエストの目的に合致しています。

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify if the new lines are covered by tests.

# Test: Search for the test coverage of the lines. Expect: Coverage report showing the lines.
rg --type rust $'pub fn join(self) -> Result<(), String>' nusamai/src/pipeline/runner.rs

Length of output: 89



Script:

#!/bin/bash
# Search for test functions that call the `join` method in the codebase.
rg 'join\(\)' --type rust --glob '*_test.rs'

# Search for test files that might include tests for `PipelineHandle`.
rg 'PipelineHandle' --type rust --glob '*_test.rs'

Length of output: 395



Script:

#!/bin/bash
# Locate all test files in the repository.
fd --type f --glob '*_test.rs'

# Search for the `join` method in the located test files.
fd --type f --glob '*_test.rs' --exec rg 'join\(\)'

# Search for `PipelineHandle` in the located test files.
fd --type f --glob '*_test.rs' --exec rg 'PipelineHandle'

Length of output: 144



Script:

#!/bin/bash
# List all files in the repository to identify potential test files.
fd --type f

# Search for the `join` method in all Rust files to ensure we are not missing any test files.
rg 'join\(\)' --type rust

# Search for `PipelineHandle` in all Rust files to ensure we are not missing any test files.
rg 'PipelineHandle' --type rust

Length of output: 58092



Script:

#!/bin/bash
# Extract and review the relevant test functions in `nusamai/tests/sink.rs` and `nusamai/tests/pipeline.rs`.

# Extract test functions from `nusamai/tests/sink.rs` that call `join`.
rg -A 10 'handle.join\(\)' nusamai/tests/sink.rs

# Extract test functions from `nusamai/tests/pipeline.rs` that call `join`.
rg -A 10 'handle.join\(\)' nusamai/tests/pipeline.rs

Length of output: 606

}
}

Expand Down
5 changes: 4 additions & 1 deletion nusamai/src/sink/shapefile/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
};

for (field_name, attr) in attributes {
let name = field_name.as_str().try_into().unwrap(); // FIXME: handle errors
let Ok(name) = field_name.as_str().try_into() else {
log::error!("Field name '{}' cannot be used in Shapefile", field_name);
continue;

Check warning on line 29 in nusamai/src/sink/shapefile/attributes.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/sink/shapefile/attributes.rs#L28-L29

Added lines #L28 - L29 were not covered by tests
};
Comment on lines +27 to +30
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

make_table_builder 関数に追加されたエラーハンドリングコードがテストでカバーされていないようです。このコードの堅牢性を確保するために、手動での確認と新しいエラーハンドリングコードをカバーするテストの追加をお勧めします。

  • nusamai/src/sink/shapefile/attributes.rsmake_table_builder 関数
Analysis chain

make_table_builder 関数に追加されたエラーハンドリングにより、無効なフィールド名が発生した場合にエラーをログに記録し、処理をスキップすることができるようになりました。この変更は、エラーハンドリングを強化するというプルリクエストの目的に合致しています。

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify if the new lines are covered by tests.

# Test: Search for the test coverage of the lines. Expect: Coverage report showing the lines.
rg --type rust $'let Ok(name) = field_name.as_str().try_into()' nusamai/src/sink/shapefile/attributes.rs

Length of output: 105



Script:

#!/bin/bash
# Description: Search for test functions that might cover the `make_table_builder` function.

# Find all test files in the repository
test_files=$(fd --type f --extension rs --exec rg --files-with-matches 'fn test' {} +)

# Search for invocations of `make_table_builder` in the test files
for file in $test_files; do
    rg 'make_table_builder' $file
done

Length of output: 14593



Script:

#!/bin/bash
# Description: Search for test functions in `nusamai/src/sink/shapefile/mod.rs` that might cover `make_table_builder`.

# Search for test functions in the file
rg 'fn test' nusamai/src/sink/shapefile/mod.rs

Length of output: 94



Script:

#!/bin/bash
# Description: Search for invocations of `make_table_builder` within the `test_toplevel_cityobj_multipolygon` function.

# Search for the `test_toplevel_cityobj_multipolygon` function definition and its body
rg -A 20 'fn test_toplevel_cityobj_multipolygon' nusamai/src/sink/shapefile/mod.rs | rg 'make_table_builder'

Length of output: 108

let key = field_name.to_string();

match attr.type_ref {
Expand Down
18 changes: 17 additions & 1 deletion nusamai/src/transformer/transform/shp_field_dict.json
Original file line number Diff line number Diff line change
Expand Up @@ -847,5 +847,21 @@
"zonalDisasterPreventionFacilities": "zoneDPFacl",
"zonalDisasterPreventionFacilitiesAllocation": "zDiPrFcAlc",
"zoneName": "zoneName",
"zoneNumber": "zone#"
"zoneNumber": "zone#",
"geometrySrcDesc0": "geomSrcD0",
"geometrySrcDesc1": "geomSrcD1",
"geometrySrcDesc2": "geomSrcD2",
"geometrySrcDesc3": "geomSrcD3",
"geometrySrcDesc4": "geomSrcD4",
"appearanceSrcDescLod0": "appSDLod0",
"appearanceSrcDescLod1": "appSDLod1",
"appearanceSrcDescLod2": "appSDLod2",
"appearanceSrcDescLod3": "appSDLod3",
"appearanceSrcDescLod4": "appSDLod4",
"tranDataAcquisition": "tranDtaAcq",
"publicSurveyDataQualityAttribute": "pubSvDQual",
"bldgUsecaseAttribute": "bldgUC",
"frnKeyValuePairAttribute": "frnKVPair",
"tranKeyValuePairAttribute": "tranKVPair",
"tranUsecaseAttribute": "tranUC"
}
1 change: 0 additions & 1 deletion nusamai/tests/mod.rs

This file was deleted.

2 changes: 1 addition & 1 deletion nusamai/tests/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,5 +162,5 @@ fn test_run_pipeline() {
});

// wait for the pipeline to finish
handle.join();
handle.join().unwrap();
}
2 changes: 1 addition & 1 deletion nusamai/tests/sink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub(crate) fn simple_run_sink<S: DataSinkProvider>(sink_provider: S, output: Opt

let (handle, watcher, canceller) =
nusamai::pipeline::run(source, transformer, sink, schema.into());
handle.join();
handle.join().unwrap();

for msg in watcher {
println!("Feedback message from the pipeline {:?}", msg);
Expand Down
Loading