Merges repeated tests by Alberto-o · Pull Request #338 · OpenSEMBA/fdtd

Alberto-o · 2026-03-25T11:31:58Z

Addresses issue #290

…ts. Fixes conflicts

Copilot

Pull request overview

This PR primarily targets test-suite deduplication in the Python wrapper integration tests (per issue #290), consolidating repeated wire/unshielded variants into single tests while adjusting a couple of JSON test cases to support the unified flows.

Changes:

Consolidates several repeated test_full_system.py cases by removing near-duplicate wire vs unshielded variants and renaming tests.
Updates test-case JSON inputs for SGBC resistance and line-integral probe scenarios (geometry/probe/source tweaks).
Adds a few new execution/assignment sanity tests in test_full_system.py (not described in the PR metadata).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.

File	Description
testData/cases/sgbcResistance/sgbcResistance.fdtd.json	Adjusts mesh elements/source attachment and adds wire probes used by the SGBC resistance scenario.
testData/cases/lineIntegralProbe/lineIntegralProbe_plates.fdtd.json	Splits a polyline into two and adds a generator attribute while extending wire material associations.
test/pyWrapper/test_full_system.py	Merges repeated tests, renames a few, removes duplicate variants, and introduces new execution/assignment tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T16:12:02Z

test/pyWrapper/test_full_system.py

+    folderWitSpaces: str  = os.path.join(tmp_path, "spaced bin")
+    os.mkdir(folderWitSpaces)
+    if platform == 'win32':
+        shutil.copy2(NGSPICE_DLL, folderWitSpaces)
+
+    sembaExecutable = SEMBA_EXE.split(os.path.sep)[-1]
+    pathToExe: str = os.path.join(folderWitSpaces, sembaExecutable)


Variable name folderWitSpaces looks like a typo (missing 'h' in 'With') and makes the test harder to read/grep. Consider renaming to folderWithSpaces (and similarly use os.path.basename(SEMBA_EXE) instead of manual split for clarity).

Suggested change

folderWitSpaces: str = os.path.join(tmp_path, "spaced bin")

os.mkdir(folderWitSpaces)

if platform == 'win32':

shutil.copy2(NGSPICE_DLL, folderWitSpaces)

sembaExecutable = SEMBA_EXE.split(os.path.sep)[-1]

pathToExe: str = os.path.join(folderWitSpaces, sembaExecutable)

folderWithSpaces: str = os.path.join(tmp_path, "spaced bin")

os.mkdir(folderWithSpaces)

if platform == 'win32':

shutil.copy2(NGSPICE_DLL, folderWithSpaces)

sembaExecutable = os.path.basename(SEMBA_EXE)

pathToExe: str = os.path.join(folderWithSpaces, sembaExecutable)

Copilot · 2026-03-27T16:12:03Z

test/pyWrapper/test_full_system.py

+def test_can_execute_fdtd_from_folder_with_spaces_and_can_process_additional_arguments(tmp_path):
+    projectRoot = os.getcwd()
+    folderWitSpaces: str  = os.path.join(tmp_path, "spaced bin")
+    os.mkdir(folderWitSpaces)
+    if platform == 'win32':
+        shutil.copy2(NGSPICE_DLL, folderWitSpaces)
+
+    sembaExecutable = SEMBA_EXE.split(os.path.sep)[-1]
+    pathToExe: str = os.path.join(folderWitSpaces, sembaExecutable)
+    shutil.copy2(SEMBA_EXE, pathToExe)
+    print(pathToExe)

-    resistanceBulkProbe = Probe( \
-    solver.getSolvedProbeFilenames("Bulk probe Resistance")[0])
-    nodalBulkProbe = Probe( \
-        solver.getSolvedProbeFilenames("Bulk probe Nodal Source")[0])
-    excitation = ExcitationFile( \
-        excitation_filename=solver.getExcitationFile("predefinedExcitation")[0])
-
-    # For debugging.
-    # plt.figure()
-    # plt.plot(resistanceBulkProbe['time'].to_numpy(), 
-    #         resistanceBulkProbe['current'].to_numpy(), label='BP Current@resistance')
-    # plt.plot(excitation.data['time'].to_numpy(), 
-    #         excitation.data['value'].to_numpy(), label='excited current')
-    # plt.plot(nodalBulkProbe['time'].to_numpy(),
-    #         -nodalBulkProbe['current'].to_numpy(), label='BP Current@nodal source')
-    # plt.legend()
-
-    exc = np.interp(nodalBulkProbe['time'].to_numpy(), 
-                    excitation.data['time'].to_numpy(), 
-                    excitation.data['value'].to_numpy())
-    assert np.corrcoef(exc, -nodalBulkProbe['current'])[0,1] > 0.999
-    assert np.corrcoef(-nodalBulkProbe['current'], resistanceBulkProbe['current'])[0,1] > 0.998
+    fn = CASES_FOLDER + "dielectric/dielectricTransmission.fdtd.json"
+    solver = FDTD(fn, path_to_exe=pathToExe, run_in_folder=tmp_path)
+    solver.run()


This test name claims it executes FDTD from a folder with spaces and verifies additional-argument handling, but the solver is run in tmp_path (no spaces) and no extra flags/arguments are passed. Either adjust the test to actually run in folderWithSpaces and/or pass representative flags, or rename the test to match what it currently validates (an executable path containing spaces).

Copilot · 2026-03-27T16:12:03Z

test/pyWrapper/test_full_system.py

+    solver = FDTD(fn, path_to_exe=pathToExe, run_in_folder=tmp_path)
+    solver.run()
+    assert (Probe(solver.getSolvedProbeFilenames("outside")[0]) is not None)
+    assert (solver.getVTKMap()[0] is not None)


FDTD.getVTKMap() returns a path string (or None), not a list. Indexing with [0] will either raise or just take the first character of the path. Also, this test never passes -mapvtk, so getVTKMap() will likely return None even when the run succeeds. Use flags=['-mapvtk'] (or the project’s equivalent) and assert solver.getVTKMap() is a non-None existing file path.

Suggested change

solver = FDTD(fn, path_to_exe=pathToExe, run_in_folder=tmp_path)

solver.run()

assert (Probe(solver.getSolvedProbeFilenames("outside")[0]) is not None)

assert (solver.getVTKMap()[0] is not None)

solver = FDTD(fn, path_to_exe=pathToExe, run_in_folder=tmp_path, flags=['-mapvtk'])

solver.run()

assert (Probe(solver.getSolvedProbeFilenames("outside")[0]) is not None)

vtk_map_path = solver.getVTKMap()

assert vtk_map_path is not None and os.path.isfile(vtk_map_path)

Copilot · 2026-03-27T16:12:03Z

test/pyWrapper/test_full_system.py

+def test_can_execute_fdtd_from_folder_with_spaces_and_can_process_additional_arguments(tmp_path):
+    projectRoot = os.getcwd()
+    folderWitSpaces: str  = os.path.join(tmp_path, "spaced bin")
+    os.mkdir(folderWitSpaces)
+    if platform == 'win32':
+        shutil.copy2(NGSPICE_DLL, folderWitSpaces)
+
+    sembaExecutable = SEMBA_EXE.split(os.path.sep)[-1]
+    pathToExe: str = os.path.join(folderWitSpaces, sembaExecutable)
+    shutil.copy2(SEMBA_EXE, pathToExe)
+    print(pathToExe)

-    resistanceBulkProbe = Probe( \
-    solver.getSolvedProbeFilenames("Bulk probe Resistance")[0])
-    nodalBulkProbe = Probe( \
-        solver.getSolvedProbeFilenames("Bulk probe Nodal Source")[0])
-    excitation = ExcitationFile( \
-        excitation_filename=solver.getExcitationFile("predefinedExcitation")[0])
-
-    # For debugging.
-    # plt.figure()
-    # plt.plot(resistanceBulkProbe['time'].to_numpy(), 
-    #         resistanceBulkProbe['current'].to_numpy(), label='BP Current@resistance')
-    # plt.plot(excitation.data['time'].to_numpy(), 
-    #         excitation.data['value'].to_numpy(), label='excited current')
-    # plt.plot(nodalBulkProbe['time'].to_numpy(),
-    #         -nodalBulkProbe['current'].to_numpy(), label='BP Current@nodal source')
-    # plt.legend()
-
-    exc = np.interp(nodalBulkProbe['time'].to_numpy(), 
-                    excitation.data['time'].to_numpy(), 
-                    excitation.data['value'].to_numpy())
-    assert np.corrcoef(exc, -nodalBulkProbe['current'])[0,1] > 0.999
-    assert np.corrcoef(-nodalBulkProbe['current'], resistanceBulkProbe['current'])[0,1] > 0.998
+    fn = CASES_FOLDER + "dielectric/dielectricTransmission.fdtd.json"
+    solver = FDTD(fn, path_to_exe=pathToExe, run_in_folder=tmp_path)
+    solver.run()
+    assert (Probe(solver.getSolvedProbeFilenames("outside")[0]) is not None)
+    assert (solver.getVTKMap()[0] is not None)


This PR is described as merging repeated tests, but this block introduces new behavioral tests (execution from a path with spaces, additional-arguments handling, and VTK map generation). If these additions are intentional, please update the PR description / linked issue scope so reviewers know this PR also expands test coverage beyond deduplication.

Copilot · 2026-03-27T16:12:04Z

test/pyWrapper/test_full_system.py

+    projectRoot = os.getcwd()
+    folderWitSpaces: str  = os.path.join(tmp_path, "spaced bin")
+    os.mkdir(folderWitSpaces)
+    if platform == 'win32':
+        shutil.copy2(NGSPICE_DLL, folderWitSpaces)
+
+    sembaExecutable = SEMBA_EXE.split(os.path.sep)[-1]
+    pathToExe: str = os.path.join(folderWitSpaces, sembaExecutable)
+    shutil.copy2(SEMBA_EXE, pathToExe)
+    print(pathToExe)



projectRoot is assigned but never used, and print(pathToExe) will add noise to test output. Please remove the unused variable and avoid printing in tests unless it’s behind a debug flag or only executed on failure.

Copilot · 2026-03-27T16:12:04Z

test/pyWrapper/test_full_system.py

+# compiled without mtln uses classic wires
+# compiled with mltn, wire is treated as an unshielded multiwire
+def test_nodal_source(tmp_path):


Typo in the comment: "mltn" should be "mtln" (the environment variable and markers in this repo use MTLN).

Copilot · 2026-03-27T16:12:05Z

testData/cases/lineIntegralProbe/lineIntegralProbe_plates.fdtd.json

+            "elementIds": [5],
+            "resistance" : 0.0


The new "resistance": 0.0 attribute on a "type": "generator" source does not appear to be parsed/used by the current JSON parser (there is no source-level resistance key handled alongside magnitudeFile/field). If the intent is to model generator internal resistance, it likely won’t have any effect; otherwise it’s confusing test data and should be removed or implemented end-to-end.

Suggested change

"elementIds": [5],

"resistance" : 0.0

"elementIds": [5]

Copilot · 2026-03-27T16:12:05Z

test/pyWrapper/test_full_system.py

+# compiled without mtln uses classic wires
+# compiled with mltn, wire is treated as an unshielded multiwire
+def test_holland(tmp_path):


Typo in the comment: "mltn" should be "mtln" (the environment variable and markers in this repo use MTLN).

Copilot · 2026-03-27T16:12:05Z

test/pyWrapper/test_full_system.py

+# compiled without mtln uses classic wires
+# compiled with mltn, wire is treated as an unshielded multiwire
 def test_sgbc_structured_resistance_single_wire(tmp_path):


Typo in the comment: "mltn" should be "mtln" (the environment variable and markers in this repo use MTLN).

Copilot · 2026-03-27T16:12:05Z

test/pyWrapper/test_full_system.py

+# compiled without mtln uses classic wires
+# compiled with mltn, wire is treated as an unshielded multiwire
+def test_sgbc_overlapping_sgbc(tmp_path):


Typo in the comment: "mltn" should be "mtln" (the environment variable and markers in this repo use MTLN).

…uggestions

Some test simplification

14b2aaf

Alberto-o self-assigned this Mar 25, 2026

Alberto-o added the Housecleaning Stuff related with style conventions, good practices, clean code, repo organization, etc. label Mar 25, 2026

Alberto-o added 6 commits March 25, 2026 12:54

Removes sgbc and nodal source repeated tests

dd3c642

updates test

bf089a8

temporarily mark skip for lineIntegralProbe

9168981

Minor in json

2f11cf2

Merge remote-tracking branch 'origin/dev' into 290-merge-repeated-tests

10a28b1

Merge remote-tracking branch 'origin/dev' into 290-merge-repeated-tes…

b72ef7f

…ts. Fixes conflicts

Alberto-o marked this pull request as ready for review March 27, 2026 15:22

Alberto-o requested review from Copilot and lmdiazangulo March 27, 2026 15:22

lmdiazangulo approved these changes Mar 27, 2026

View reviewed changes

Copilot started reviewing on behalf of Alberto-o March 27, 2026 16:02 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Fiexes json (had been changed during tests). Addresses some copilot s…

504181d

…uggestions

Alberto-o merged commit 699c339 into dev Mar 27, 2026
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merges repeated tests#338

Merges repeated tests#338
Alberto-o merged 8 commits intodevfrom
290-merge-repeated-tests

Alberto-o commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Alberto-o commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants