hotfix: Shapefile 用のフィールド名辞書を更新する #545
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthroughこの変更は、複数のファイルにおいてエラーハンドリングの強化を目的としています。特に、 Changes
Tip Early Access Features
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
ciscorn
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
nusamai/src/transformer/transform/shp_field_dict.jsonis excluded by!**/*.json
Files selected for processing (6)
- app/src-tauri/src/main.rs (1 hunks)
- nusamai/src/main.rs (1 hunks)
- nusamai/src/pipeline/runner.rs (1 hunks)
- nusamai/src/sink/shapefile/attributes.rs (1 hunks)
- nusamai/tests/pipeline.rs (1 hunks)
- nusamai/tests/sink.rs (1 hunks)
Additional Context Used
GitHub Check Runs (1)
codecov/patch failure (8)
nusamai/src/main.rs: [warning] 287-287: nusamai/src/main.rs#L287
Added line #L287 was not covered by tests
nusamai/src/pipeline/runner.rs: [warning] 114-118: nusamai/src/pipeline/runner.rs#L114-L118
Added lines #L114 - L118 were not covered by tests
nusamai/src/pipeline/runner.rs: [warning] 120-120: nusamai/src/pipeline/runner.rs#L120
Added line #L120 was not covered by tests
nusamai/src/pipeline/runner.rs: [warning] 122-123: nusamai/src/pipeline/runner.rs#L122-L123
Added lines #L122 - L123 were not covered by tests
nusamai/src/pipeline/runner.rs: [warning] 127-127: nusamai/src/pipeline/runner.rs#L127
Added line #L127 was not covered by tests
nusamai/src/pipeline/runner.rs: [warning] 130-130: nusamai/src/pipeline/runner.rs#L130
Added line #L130 was not covered by tests
nusamai/src/pipeline/runner.rs: [warning] 133-133: nusamai/src/pipeline/runner.rs#L133
Added line #L133 was not covered by tests
nusamai/src/sink/shapefile/attributes.rs: [warning] 28-29: nusamai/src/sink/shapefile/attributes.rs#L28-L29
Added lines #L28 - L29 were not covered by tests
Additional comments not posted (2)
nusamai/tests/sink.rs (1)
67-67:handle.join().unwrap();の追加により、スレッドの結合中に発生する可能性のあるエラーを適切に処理できるようになりました。この変更は、エラーハンドリングを強化するというプルリクエストの目的に合致しています。Verification successful
handle.join().unwrap();の追加により、スレッドの結合中に発生する可能性のあるエラーを適切に処理できるようになりました。この変更は、エラーハンドリングを強化するというプルリクエストの目的に合致しています。simple_run_sink関数がテストファイル内で複数回呼び出されているため、この新しい行もテストでカバーされています。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 $'handle.join().unwrap();' nusamai/tests/sink.rsLength of output: 64
Script:
#!/bin/bash # Description: Verify if the function `simple_run_sink` is invoked in any test cases. # Test: Search for invocations of `simple_run_sink` in the test files. rg 'simple_run_sink' nusamai/tests/Length of output: 1066
nusamai/tests/pipeline.rs (1)
165-165:handle.join().unwrap();の追加により、スレッドの結合中に発生する可能性のあるエラーを適切に処理できるようになりました。この変更は、エラーハンドリングを強化するというプルリクエストの目的に合致しています。Verification successful
handle.join().unwrap();の追加により、スレッドの結合中に発生する可能性のあるエラーを適切に処理できるようになりました。この変更は、エラーハンドリングを強化するというプルリクエストの目的に合致しています。また、この行はtest_run_pipelineテスト関数内に含まれているため、テストによってカバーされています。
nusamai/tests/pipeline.rsのtest_run_pipeline関数内の165行目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 $'handle.join().unwrap();' nusamai/tests/pipeline.rsLength of output: 68
Script:
#!/bin/bash # Description: Find all test functions in the file and check if they cover the new line. # Find all test functions in the file rg --type rust '#\[test\]' -A 1 nusamai/tests/pipeline.rs # Check if any test functions call `handle.join().unwrap();` rg --type rust 'handle.join().unwrap();' nusamai/tests/pipeline.rs -A 10Length of output: 167
Script:
#!/bin/bash # Description: Extract the content of the `test_run_pipeline` function to verify if it includes the new line. # Extract the `test_run_pipeline` function ast-grep --lang rust --pattern $'fn test_run_pipeline() { $$$ }' nusamai/tests/pipeline.rsLength of output: 3088
Comment on lines
+113
to
+135
| 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) | ||
| } else { | ||
| format!("{stage} thread panicked with an unknown type.") | ||
| }; | ||
| log::error!("{}", msg); | ||
| msg.to_string() | ||
| } | ||
| 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)); | ||
| } | ||
| if let Err(err) = self.transformer_thread_handle.join() { | ||
| return Err(report_error("Transformer", err)); | ||
| } | ||
| 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)); | ||
| } | ||
| Ok(()) |
There was a problem hiding this comment.
Tip
Codebase Verification
join 関数の新しいエラーハンドリングロジックがテストでカバーされているか確認するために、テストファイル nusamai/tests/sink.rs と nusamai/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
Comment on lines
+27
to
+30
| let Ok(name) = field_name.as_str().try_into() else { | ||
| log::error!("Field name '{}' cannot be used in Shapefile", field_name); | ||
| continue; | ||
| }; |
There was a problem hiding this comment.
Tip
Codebase Verification
make_table_builder 関数に追加されたエラーハンドリングコードがテストでカバーされていないようです。このコードの堅牢性を確保するために、手動での確認と新しいエラーハンドリングコードをカバーするテストの追加をお勧めします。
nusamai/src/sink/shapefile/attributes.rsのmake_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
Comment on lines
+286
to
+289
| if let Err(msg) = handle.join() { | ||
| log::error!("Pipeline thread panicked: {:?}", msg); | ||
| } | ||
|
|
There was a problem hiding this comment.
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
Comment on lines
+263
to
+267
| if let Err(msg) = handle.join() { | ||
| return Err(Error::ConversionFailed(format!( | ||
| "Pipeline thread panicked: {msg}" | ||
| ))); | ||
| } |
There was a problem hiding this comment.
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
nokonoko1203
added a commit
that referenced
this pull request
May 31, 2024
commit 0f7b652 Author: satoshi komatsu <[email protected]> Date: Thu May 30 17:33:09 2024 +0900 minecraft-sinkの実装 (#528) <!-- Close or Related Issues --> minecraft-sinkを実装しました。以下の理由によりイシューは閉じません。 - 出力時に複数のLODがでてしまう不具合があり。(#532) - コマンド引数のオプションの未実装。 - テスト未実装 <!-- Please describe the motivation behind this PR and the changes it introduces. --> <!-- 何のために、どのような変更をしますか？ --> - `.mca`ファイルを出力するsinkを実装 - 出力時は`region`フォルダが作成され、その中に複数の`.mca`を生成 - 変換時の座標系は`ExtendedTransverseMercatorProjection`で横メルカトル座標系に変換し、citygmlの全体の中心がマインクラフトのワールドの中心に来るように設定 - ブロックの高さはcitygmlのそのままの高さにしているため、標高が高すぎる建物等は現状変換しても表示されない（マイクラのブロックの設置可能高さは `-64` ~ `319`） - ブロックの種類は地物型の種類によって分けるように実装。また、ボクセライザー側でRGBのカラー情報を受け取るように修正。 | 地物型 | ブロック | RGB | |--------------------------------------|---------------------------------------|-----------------| | bldg:Building | minecraft:iron_block | 220, 220, 220 | | tran:Road | minecraft:gray_wool | 62, 68, 71 | | tran:Railway | minecraft:granite | 149, 103, 85 | | tran:Track | minecraft:stone_bricks | 122, 121, 122 | | tran:Square | minecraft:smooth_stone | 158, 158, 158 | | uro:Waterway | minecraft:cyan_stained_glass | 75, 127, 153 | | luse:LandUse | minecraft:coarse_dirt | 119, 85, 59 | | frn:CityFurniture | minecraft:quartz_block | 235, 229, 222 | | veg:PlantCover | minecraft:moss_block | 89, 109, 45 | | veg:SolitaryVegetationObject | minecraft:oak_leaves | 0, 255, 0 | | wtr:WaterBody | minecraft:water | 0, 0, 255 | | dem:ReliefFeature | minecraft:stone | 125, 125, 125 | | brid:Bridge | minecraft:polished_andesite | 132, 134, 133 | | tun:Tunnel | minecraft:cobblestone | 127, 127, 127 | | urf:UseDistrict | minecraft:green_stained_glass | 102, 127, 50 | | urf:FirePreventionDistrict | minecraft:red_stained_glass | 53, 50, 50 | | urf:SedimentDisasterProneArea | minecraft:yellow_stained_glass | 229, 229, 50 | | urf:Zone | minecraft:magenta_stained_glass | 178, 75, 215 | | その他未定義の地物型 | minecraft:white_wool | 255, 255, 255 | <!-- If manual testing is required, please describe the procedure. --> <!-- 手動での動作確認が必要なら、そのやり方を記述してください。--> 下記のコマンドで出力されます ``` <input_path_gml> --sink minecraft --output <output_path> ``` `region`フォルダが出力されるので、これをマインクラフトの既存のワールドデータにある`region`ディレクトリと置き換えてワールドに入れば確認できます。（<output_path>の部分を直接マインクラフトのワールドデータのディレクトリに指定しても良い） 建築物 bldg:Building minecraft:iron_block  道路 tran:Road minecraft:gray_wool  鉄道 tran:Railway minecraft:granite  歩道 tran:Track minecraft:stone_bricks  広場 tran:Square minecraft:smooth_stone  航路 uro:Waterway minecraft:cyan_stained_glass  土地利用 luse:LandUse minecraft:coarse_dirt  用途地域 urf:UseDistrict green_stained_glass  防火地域又は準防火地域 urf:FirePreventionDistrict green_stained_glass  土砂災害警戒区域 minecraft:yellow_stained_glass urf:SedimentDisasterProneArea  都市設備 frn:CityFurniture minecraft:quartz_block  植被 veg:PlantCover minecraft:moss_block  単独木 veg:SolitaryVegetationObject minecraft:oak_leaves  洪水浸水想定区域、津波浸水想定、高潮浸水想定区域、内水浸水想定区域、海、潮汐水域、水路、河川/小川、湖、滝、湿地・沼地、浸水域、貯水池、不明 wtr:WaterBody minecraft:water  地形 dem:ReliefFeature minecraft:stone  橋梁 brid:Bridge minecraft:polished_andesite  トンネル tun:Tunnel minecraft:cobblestone  区域 urf:Zone minecraft:magenta_stained_glass  --------- Co-authored-by: nokonoko1203 <[email protected]> Co-authored-by: Taku Fukada <[email protected]> commit 0c6addf Author: Taku Fukada <[email protected]> Date: Wed May 29 15:49:36 2024 +0900 Publish nusamai-geometry as 'flatgeom' on crates.io. (#546) Close: #536 Publish nusamai-geometry as [flatgeom](https://crates.io/crates/flatgeom) on crates.io commit 720b2f0 Author: Taku Fukada <[email protected]> Date: Wed May 29 15:38:17 2024 +0900 Fix: DisasterRiskAttributeProperty for backward compatibility (#547) PLATEAU 4.0において `uro:BuildingHighTideRiskAttribute`（建築物で使用） と `uro:HighTideRiskAttribute` （その他の地物で使用）が1つの `uro:HighTideRiskAttribute` に統一されたため #524 で対応したが、後方互換性のために `uro:BuildingHighTideRiskAttribute` 要素も拾うような対処はしていなかった。対応する。 commit 1cd36a8 Author: Taku Fukada <[email protected]> Date: Tue May 28 23:57:00 2024 +0900 hotfix: Shapefile 用のフィールド名辞書を更新する (#545) PLATEAU 4.0 対応 (#524) により、出現するフィールド名の種類が増えたが、Shapefile 用の属性名変換辞書を辞書を更新していなかった。また、11文字以上のフィールド名が含まれていると panic する実装になっていた。 - これらを修正する。 - 上記の問題をテストで検出できるようにするため、パイプライン全体を最後に join する際のエラーを捕捉するようにする。現在はパイプライン内のスレッドが panic してもテストが通ることがある。 commit 9f6c373 Author: Taku Fukada <[email protected]> Date: Tue May 28 10:30:43 2024 +0900 外部ソートを独自の実装に切り替える (#531) Close #302 外部ソートライブラリを、独自に実装した [kv-extsort](https://github.com/MIERUNE/kv-extsort-rs) に変更する。 [ext-sort](https://crates.io/crates/ext-sort) は必要なくなる。 - ソート中でも処理をキャンセルできるようになる。 - パフォーマンスがいくらか向上する。 - 信頼性は低下する、かもしれない → kv-extsort側の品質を上げていく Not required / 不要
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PLATEAU 4.0 対応 (#524) により、出現するフィールド名の種類が増えたが、Shapefile 用の属性名変換辞書を辞書を更新していなかった。また、11文字以上のフィールド名が含まれていると即 panic する実装であった。