From 831354a0db24d598b530ec5a9e7f5025a3f2e46f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20H=C3=A6gland?= Date: Mon, 22 Sep 2025 20:46:10 +0200 Subject: [PATCH 1/2] Fix race condition in Python tests by disabling async ECL output Add --enable-async-ecl-output=false to all Python simulator tests to prevent race condition between async file writing tasklets and pushd context manager. Problem: - Tests use pushd() to change directory before running simulations - Simulator dispatches async tasklets to write ECL output files - pushd context exits before async writes complete - Tasklets write files in wrong directory (build/python/ instead of test_data/) - Causes confusing error messages and misplaced output files Solution: - Disable async ECL output during tests for deterministic behavior - Files now created in correct locations within test_data/ directories - Eliminates "Can not open EclFile" errors in logs - Makes test execution predictable and synchronous --- python/test/test_basic.py | 4 ++-- python/test/test_fluidstate_variables.py | 4 ++-- python/test/test_mpi.py | 4 ++-- python/test/test_primary_variables.py | 4 ++-- python/test/test_schedule.py | 2 +- python/test/test_throw.py | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/python/test/test_basic.py b/python/test/test_basic.py index 95ae49035ef..95fafea5383 100755 --- a/python/test/test_basic.py +++ b/python/test/test_basic.py @@ -21,7 +21,7 @@ def setUpClass(cls): # IMPORTANT: This test must be run first since it calls MPI_Init() def test_01_blackoil(self): with pushd(self.data_dir_bo): - sim = BlackOilSimulator(args=['--linear-solver=ilu0'], filename="SPE1CASE1.DATA") + sim = BlackOilSimulator(args=['--linear-solver=ilu0', '--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") sim.setup_mpi(init=True, finalize=False) sim.step_init() sim.step() @@ -48,7 +48,7 @@ def test_01_blackoil(self): # IMPORTANT: This test must be run last since it calls MPI_Finalize() def test_99_gaswater(self): with pushd(self.data_dir_gw): - sim = GasWaterSimulator(args=['--linear-solver=ilu0'], filename="SPE1CASE2_GASWATER.DATA") + sim = GasWaterSimulator(args=['--linear-solver=ilu0', '--enable-async-ecl-output=false'], filename="SPE1CASE2_GASWATER.DATA") sim.setup_mpi(init=False, finalize=True) sim.step_init() sim.step() diff --git a/python/test/test_fluidstate_variables.py b/python/test/test_fluidstate_variables.py index 30bc8dbd6a0..45a98d99f16 100644 --- a/python/test/test_fluidstate_variables.py +++ b/python/test/test_fluidstate_variables.py @@ -21,7 +21,7 @@ def setUpClass(cls): # IMPORTANT:This test must be run first since it calls MPI_Init() def test_01_blackoil(self): with pushd(self.data_dir_bo): - sim = BlackOilSimulator("SPE1CASE1.DATA") + sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") sim.setup_mpi(True, False) sim.step_init() sim.step() @@ -53,7 +53,7 @@ def test_01_blackoil(self): # IMPORTANT: This test must be run last since it calls MPI_Finalize() def test_99_gaswater(self): with pushd(self.data_dir_gw): - sim = GasWaterSimulator("SPE1CASE2_GASWATER.DATA") + sim = GasWaterSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE2_GASWATER.DATA") sim.setup_mpi(False, True) sim.step_init() sim.step() diff --git a/python/test/test_mpi.py b/python/test/test_mpi.py index 9d4d91f41b1..cbd77f7f8f6 100644 --- a/python/test/test_mpi.py +++ b/python/test/test_mpi.py @@ -18,14 +18,14 @@ def setUpClass(cls): # are run in a given order. def test_01_mpi_init(self): with pushd(self.data_dir): - sim = BlackOilSimulator("SPE1CASE1.DATA") + sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") sim.setup_mpi(init=True, finalize=False) sim.step_init() # This will create the OPM::Main() object which will call MPI_Init() assert True def test_02_mpi_no_init(self): with pushd(self.data_dir): - sim = BlackOilSimulator("SPE1CASE1.DATA") + sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") sim.setup_mpi(init=False, finalize=True) sim.step_init() # This will create the OPM::Main() object which will not call MPI_Init() # That this test runs shows that the simulator does not call diff --git a/python/test/test_primary_variables.py b/python/test/test_primary_variables.py index f481c094d91..5c876db5e7f 100644 --- a/python/test/test_primary_variables.py +++ b/python/test/test_primary_variables.py @@ -24,7 +24,7 @@ def setUpClass(cls): # IMPORTANT:This test must be run first since it calls MPI_Init() def test_01_blackoil(self): with pushd(self.data_dir_bo): - sim = BlackOilSimulator("SPE1CASE1.DATA") + sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") sim.setup_mpi(True, False) sim.step_init() sim.step() @@ -54,7 +54,7 @@ def test_01_blackoil(self): # IMPORTANT: This test must be run last since it calls MPI_Finalize() def test_99_gaswater(self): with pushd(self.data_dir_gw): - sim = GasWaterSimulator("SPE1CASE2_GASWATER.DATA") + sim = GasWaterSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE2_GASWATER.DATA") sim.setup_mpi(False, True) sim.step_init() sim.step() diff --git a/python/test/test_schedule.py b/python/test/test_schedule.py index f811b1ff1d0..f94c82e3bf5 100755 --- a/python/test/test_schedule.py +++ b/python/test/test_schedule.py @@ -30,7 +30,7 @@ def test_all(self): self.assertEqual(dt.datetime(2015, 1, 1), self.schedule.start) self.assertEqual(dt.datetime(2016, 1, 1), self.schedule.end) self.sim = BlackOilSimulator( - self.deck, state, self.schedule, summary_config ) + self.deck, state, self.schedule, summary_config, args=['--enable-async-ecl-output=false']) tsteps = self.schedule.timesteps self.assertEqual(dt.datetime(2015, 1, 1), tsteps[0]) last_step = len(tsteps) - 1 diff --git a/python/test/test_throw.py b/python/test/test_throw.py index 73513d10cc2..af38f4b0382 100644 --- a/python/test/test_throw.py +++ b/python/test/test_throw.py @@ -12,7 +12,7 @@ def setUpClass(cls): def test_all(self): with pushd(self.data_dir): - sim = BlackOilSimulator("SPE1CASE1.DATA") + sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") # NOTE: The following call should throw an exception since the simulation # has not been initialized with self.assertRaises(RuntimeError): From c195df978d6345ba026bc0c96615d4fe425abe26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20H=C3=A6gland?= Date: Tue, 23 Sep 2025 05:09:20 +0200 Subject: [PATCH 2/2] Refactor: Use factory fixtures for simulator creation in tests Move simulator instantiation to factory functions that automatically disable async ECL output. This prevents future contributors from forgetting the --enable-async-ecl-output=false flag. - Add create_black_oil_simulator/create_gas_water_simulator to pytest_common - Define ENABLE_ASYNC_ECL_OUTPUT_FLAG constant - Update all tests to use factory functions - No functional changes, pure refactoring --- python/test/pytest_common.py | 51 ++++++++++++++++++++++++ python/test/test_basic.py | 7 ++-- python/test/test_fluidstate_variables.py | 7 ++-- python/test/test_mpi.py | 7 ++-- python/test/test_primary_variables.py | 7 ++-- python/test/test_schedule.py | 6 +-- python/test/test_throw.py | 5 +-- 7 files changed, 68 insertions(+), 22 deletions(-) diff --git a/python/test/pytest_common.py b/python/test/pytest_common.py index 3ae9c21ff8c..b190e4f3502 100644 --- a/python/test/pytest_common.py +++ b/python/test/pytest_common.py @@ -10,3 +10,54 @@ def pushd(path): yield os.chdir(cwd) +ENABLE_ASYNC_ECL_OUTPUT_FLAG = '--enable-async-ecl-output=false' + +def create_black_oil_simulator(*args, **kwargs): + """Create BlackOilSimulator with test-safe default arguments. + + Automatically disables async ECL output to prevent race conditions + with pushd context manager in tests. + """ + from opm.simulators import BlackOilSimulator + + flag_to_add = ENABLE_ASYNC_ECL_OUTPUT_FLAG + # Handle different constructor patterns + if 'args' in kwargs: + # Add our flag to existing args + if flag_to_add not in kwargs['args']: + kwargs['args'].append(flag_to_add) + elif len(args) >= 4: + # Constructor with deck, state, schedule, summary_config + # Add args parameter + if len(args) == 4: + args = args + ([flag_to_add],) + elif len(args) == 5: + # Args already provided, add our flag + args_list = list(args[4]) if args[4] else [] + if flag_to_add not in args_list: + args_list.append(flag_to_add) + args = args[:4] + (args_list,) + else: + # Constructor with filename - add args parameter + kwargs['args'] = kwargs.get('args', []) + if flag_to_add not in kwargs['args']: + kwargs['args'].append(flag_to_add) + + return BlackOilSimulator(*args, **kwargs) + +def create_gas_water_simulator(*args, **kwargs): + """Create GasWaterSimulator with test-safe default arguments. + + Automatically disables async ECL output to prevent race conditions + with pushd context manager in tests. + """ + from opm.simulators import GasWaterSimulator + + flag_to_add = ENABLE_ASYNC_ECL_OUTPUT_FLAG + # Add our flag to args + kwargs['args'] = kwargs.get('args', []) + if flag_to_add not in kwargs['args']: + kwargs['args'].append(flag_to_add) + + return GasWaterSimulator(*args, **kwargs) + diff --git a/python/test/test_basic.py b/python/test/test_basic.py index 95fafea5383..865a6514f9d 100755 --- a/python/test/test_basic.py +++ b/python/test/test_basic.py @@ -1,8 +1,7 @@ import os import unittest from pathlib import Path -from opm.simulators import BlackOilSimulator, GasWaterSimulator -from .pytest_common import pushd +from .pytest_common import pushd, create_black_oil_simulator, create_gas_water_simulator class TestBasic(unittest.TestCase): @classmethod @@ -21,7 +20,7 @@ def setUpClass(cls): # IMPORTANT: This test must be run first since it calls MPI_Init() def test_01_blackoil(self): with pushd(self.data_dir_bo): - sim = BlackOilSimulator(args=['--linear-solver=ilu0', '--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") + sim = create_black_oil_simulator(args=['--linear-solver=ilu0'], filename="SPE1CASE1.DATA") sim.setup_mpi(init=True, finalize=False) sim.step_init() sim.step() @@ -48,7 +47,7 @@ def test_01_blackoil(self): # IMPORTANT: This test must be run last since it calls MPI_Finalize() def test_99_gaswater(self): with pushd(self.data_dir_gw): - sim = GasWaterSimulator(args=['--linear-solver=ilu0', '--enable-async-ecl-output=false'], filename="SPE1CASE2_GASWATER.DATA") + sim = create_gas_water_simulator(args=['--linear-solver=ilu0'], filename="SPE1CASE2_GASWATER.DATA") sim.setup_mpi(init=False, finalize=True) sim.step_init() sim.step() diff --git a/python/test/test_fluidstate_variables.py b/python/test/test_fluidstate_variables.py index 45a98d99f16..be7bae7d82d 100644 --- a/python/test/test_fluidstate_variables.py +++ b/python/test/test_fluidstate_variables.py @@ -1,8 +1,7 @@ import os import unittest from pathlib import Path -from opm.simulators import BlackOilSimulator, GasWaterSimulator -from .pytest_common import pushd +from .pytest_common import pushd, create_black_oil_simulator, create_gas_water_simulator class TestBasic(unittest.TestCase): @classmethod @@ -21,7 +20,7 @@ def setUpClass(cls): # IMPORTANT:This test must be run first since it calls MPI_Init() def test_01_blackoil(self): with pushd(self.data_dir_bo): - sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") + sim = create_black_oil_simulator(filename="SPE1CASE1.DATA") sim.setup_mpi(True, False) sim.step_init() sim.step() @@ -53,7 +52,7 @@ def test_01_blackoil(self): # IMPORTANT: This test must be run last since it calls MPI_Finalize() def test_99_gaswater(self): with pushd(self.data_dir_gw): - sim = GasWaterSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE2_GASWATER.DATA") + sim = create_gas_water_simulator(filename="SPE1CASE2_GASWATER.DATA") sim.setup_mpi(False, True) sim.step_init() sim.step() diff --git a/python/test/test_mpi.py b/python/test/test_mpi.py index cbd77f7f8f6..93f9092344c 100644 --- a/python/test/test_mpi.py +++ b/python/test/test_mpi.py @@ -1,8 +1,7 @@ import os import unittest from pathlib import Path -from opm.simulators import BlackOilSimulator -from .pytest_common import pushd +from .pytest_common import pushd, create_black_oil_simulator class TestBasic(unittest.TestCase): @classmethod @@ -18,14 +17,14 @@ def setUpClass(cls): # are run in a given order. def test_01_mpi_init(self): with pushd(self.data_dir): - sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") + sim = create_black_oil_simulator(filename="SPE1CASE1.DATA") sim.setup_mpi(init=True, finalize=False) sim.step_init() # This will create the OPM::Main() object which will call MPI_Init() assert True def test_02_mpi_no_init(self): with pushd(self.data_dir): - sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") + sim = create_black_oil_simulator(filename="SPE1CASE1.DATA") sim.setup_mpi(init=False, finalize=True) sim.step_init() # This will create the OPM::Main() object which will not call MPI_Init() # That this test runs shows that the simulator does not call diff --git a/python/test/test_primary_variables.py b/python/test/test_primary_variables.py index 5c876db5e7f..60558ed8b7b 100644 --- a/python/test/test_primary_variables.py +++ b/python/test/test_primary_variables.py @@ -1,8 +1,7 @@ import os import unittest from pathlib import Path -from opm.simulators import BlackOilSimulator, GasWaterSimulator -from .pytest_common import pushd +from .pytest_common import pushd, create_black_oil_simulator, create_gas_water_simulator class TestBasic(unittest.TestCase): @classmethod @@ -24,7 +23,7 @@ def setUpClass(cls): # IMPORTANT:This test must be run first since it calls MPI_Init() def test_01_blackoil(self): with pushd(self.data_dir_bo): - sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") + sim = create_black_oil_simulator(filename="SPE1CASE1.DATA") sim.setup_mpi(True, False) sim.step_init() sim.step() @@ -54,7 +53,7 @@ def test_01_blackoil(self): # IMPORTANT: This test must be run last since it calls MPI_Finalize() def test_99_gaswater(self): with pushd(self.data_dir_gw): - sim = GasWaterSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE2_GASWATER.DATA") + sim = create_gas_water_simulator(filename="SPE1CASE2_GASWATER.DATA") sim.setup_mpi(False, True) sim.step_init() sim.step() diff --git a/python/test/test_schedule.py b/python/test/test_schedule.py index f94c82e3bf5..13085ef2b24 100755 --- a/python/test/test_schedule.py +++ b/python/test/test_schedule.py @@ -3,7 +3,7 @@ import datetime as dt from pathlib import Path import re -from opm.simulators import BlackOilSimulator +from .pytest_common import create_black_oil_simulator from opm.io.parser import Parser from opm.io.ecl_state import EclipseState from opm.io.schedule import Schedule @@ -29,8 +29,8 @@ def test_all(self): self.assertTrue('INJ' in self.schedule) self.assertEqual(dt.datetime(2015, 1, 1), self.schedule.start) self.assertEqual(dt.datetime(2016, 1, 1), self.schedule.end) - self.sim = BlackOilSimulator( - self.deck, state, self.schedule, summary_config, args=['--enable-async-ecl-output=false']) + self.sim = create_black_oil_simulator( + self.deck, state, self.schedule, summary_config) tsteps = self.schedule.timesteps self.assertEqual(dt.datetime(2015, 1, 1), tsteps[0]) last_step = len(tsteps) - 1 diff --git a/python/test/test_throw.py b/python/test/test_throw.py index af38f4b0382..779bb3a83e8 100644 --- a/python/test/test_throw.py +++ b/python/test/test_throw.py @@ -1,8 +1,7 @@ import os import unittest from pathlib import Path -from opm.simulators import BlackOilSimulator -from .pytest_common import pushd +from .pytest_common import pushd, create_black_oil_simulator class TestBasic(unittest.TestCase): @classmethod @@ -12,7 +11,7 @@ def setUpClass(cls): def test_all(self): with pushd(self.data_dir): - sim = BlackOilSimulator(args=['--enable-async-ecl-output=false'], filename="SPE1CASE1.DATA") + sim = create_black_oil_simulator(filename="SPE1CASE1.DATA") # NOTE: The following call should throw an exception since the simulation # has not been initialized with self.assertRaises(RuntimeError):