fix(test): clean up PEC block test marks#142
Conversation
PEC block boolean pipeline crashes in gmsh (Unknown OpenCASCADE entity). Tests were incorrectly skip-marked — they catch a real bug. xfail records the expected failure until the gmsh integration is fixed.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the TestPECBlockMesh test suite by replacing @pytest.mark.skip with @pytest.mark.xfail to track a known gmsh crash and modifies a layer name in the pec_block_sim fixture. Feedback suggests that changing the layer name to metal2 might cause a 'layer not found' error, potentially masking the intended crash. Additionally, because the crash occurs within a fixture, it may result in a test ERROR rather than an XFAIL, so moving the crashing call into the test functions was recommended.
| sim.add_cpw_port("o1", layer="metal1", s_width=20, gap_width=15, length=5.0) | ||
| sim.add_cpw_port("o2", layer="metal1", s_width=20, gap_width=15, length=5.0) | ||
| sim.add_pec(gds_layer=PEC_LAYER, from_layer="metal1", to_layer="topmetal2") | ||
| sim.add_pec(gds_layer=PEC_LAYER, from_layer="metal1", to_layer="metal2") |
There was a problem hiding this comment.
The layer name was changed from topmetal2 to metal2. However, topmetal2 is used as the default layer name in _make_sim (line 58) and was previously used here. If the default stack created by set_stack (line 398) uses topmetal2, this change might cause the test to fail with a "layer not found" error during configuration, which would prevent the test from reaching the intended gmsh crash point. Please verify if metal2 is indeed the correct layer name in this context.
| sim.add_pec(gds_layer=PEC_LAYER, from_layer="metal1", to_layer="metal2") | |
| sim.add_pec(gds_layer=PEC_LAYER, from_layer="metal1", to_layer="topmetal2") |
| @pytest.mark.xfail( | ||
| reason="PEC block surfaces cause gmsh boolean pipeline crash", | ||
| strict=False, | ||
| ) |
There was a problem hiding this comment.
Applying @pytest.mark.xfail to the class is a good way to track known issues. However, note that the crash described in the reason occurs during sim.mesh() inside the pec_block_sim fixture (line 403). In pytest, a failure or crash within a fixture typically results in an ERROR status for the tests rather than an XFAIL. If you want these tests to specifically report as XFAIL, you might need to move the crashing sim.mesh() call into the test functions themselves or handle the fixture failure explicitly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #142 +/- ##
==========================================
+ Coverage 55.55% 56.18% +0.63%
==========================================
Files 59 59
Lines 6966 6966
Branches 1270 1270
==========================================
+ Hits 3870 3914 +44
+ Misses 2697 2642 -55
- Partials 399 410 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
xfailinstead ofskip— they catch a real gmsh OpenCASCADE crash, not a known-passing featureDetails
PEC block boolean pipeline crashes in gmsh (Unknown OpenCASCADE entity). Tests were incorrectly
skip-marked.xfailrecords the expected failure until the gmsh integration is fixed.Standalone change, no dependencies on other PRs.