chore: use builders for ltp tests #711

frodehk · 2024-11-22T14:07:08Z

ECALC-1813

Why is this pull request needed?

Replace dtos with new yaml builders in ltp tests.

What does this pull request change?

Replace all dtos in ltp tests. Delete unused code.

Issues related to this change:

https://equinor-ecalc.atlassian.net/browse/ECALC-1813?atlOrigin=eyJpIjoiMzQ3NWRmMWExMjYyNDA3ZWE0N2Q0OWU3YmNiNWM2NTciLCJwIjoiaiJ9

ECALC-1813

jsolaas · 2024-11-25T08:59:54Z

src/libecalc/presentation/yaml/resource_service.py

+class DirectResourceService(ResourceService):
+    def __init__(self, resources):
+        self.resources = resources
+
+    def get_resources(self, configuration: YamlValidator) -> dict[str, MemoryResource]:
+        return self.resources


Move to testing?

jsolaas · 2024-11-25T09:04:52Z

tests/libecalc/presentation/exporter/test_ltp.py


+class OverridableStreamConfigurationService(ConfigurationService):


jsolaas · 2024-11-25T09:06:10Z

tests/libecalc/presentation/exporter/memory_resources.py

Rename or move to conftest.py so we don't have to import the fixtures where they are used.

jsolaas · 2024-11-25T09:08:59Z

tests/libecalc/presentation/exporter/memory_resources.py

+def generator_electricity2fuel_17MW_resource():
+    return MemoryResource(
+        data=[
+            [0, 0.1, 10, 11, 12, 14, 15, 16, 17, 17.1, 18.5, 20, 20.5, 20.6, 24, 28, 30, 32, 34, 36, 38, 40, 41, 410],
+            [
+                0,
+                75803.4,
+                75803.4,
+                80759.1,
+                85714.8,
+                95744,
+                100728.8,
+                105676.9,
+                110598.4,
+                136263.4,
+                143260,
+                151004.1,
+                153736.5,
+                154084.7,
+                171429.6,
+                191488,
+                201457.5,
+                211353.8,
+                221196.9,
+                231054,
+                241049.3,
+                251374.6,
+                256839.4,
+                2568394,
+            ],
+        ],  # float and int with equal value should count as equal.
+        headers=[
+            "POWER",
+            "FUEL",
+        ],
+    )


Add a momory_resource_factory in top-level conftest? taking headers and data/columns as input.

It could also be useful to create factories for the specific resources also, so we end up not duplicating the header names (which might change).

This fixture would then only have to specify power and fuel data into a electricity2fuel_factory

jsolaas · 2024-11-25T09:09:17Z

tests/libecalc/presentation/exporter/conftest.py

+
+
+@pytest.fixture
+def diesel():


Suggested change

def diesel():

def diesel_factory():

jsolaas · 2024-11-25T09:09:43Z

tests/libecalc/presentation/exporter/conftest.py

+def energy_usage_model_direct():
+    def energy_usage_model(rate: float):
+        return (
+            YamlEnergyUsageModelDirectBuilder()
+            .with_fuel_rate(rate)
+            .with_consumption_rate_type(ConsumptionRateType.STREAM_DAY)
+        ).validate()
+
+    return energy_usage_model
+
+
+@pytest.fixture
+def energy_usage_model_direct_load():
+    def energy_usage_model(load: float):
+        return (
+            YamlEnergyUsageModelDirectBuilder()
+            .with_load(load)
+            .with_consumption_rate_type(ConsumptionRateType.STREAM_DAY)
+        ).validate()
+
+    return energy_usage_model
+
+
+@pytest.fixture
+def fuel_consumer_direct(energy_usage_model_direct):
+    def fuel_consumer(
+        fuel_reference_name: str,
+        rate: float,
+        name: str = "fuel_consumer",
+        category: ConsumerUserDefinedCategoryType = ConsumerUserDefinedCategoryType.FLARE,
+    ):
+        return (
+            YamlFuelConsumerBuilder()
+            .with_name(name)
+            .with_fuel(fuel_reference_name)
+            .with_category(category)
+            .with_energy_usage_model(energy_usage_model_direct(rate))
+        ).validate()
+
+    return fuel_consumer
+
+
+@pytest.fixture
+def fuel_consumer_direct_load(energy_usage_model_direct_load):
+    def fuel_consumer(fuel_reference_name: str, load: float):
+        return (
+            YamlFuelConsumerBuilder()
+            .with_name("fuel_consumer")
+            .with_fuel(fuel_reference_name)
+            .with_category(ConsumerUserDefinedCategoryType.BASE_LOAD)
+            .with_energy_usage_model(energy_usage_model_direct_load(load))
+        ).validate()
+
+    return fuel_consumer
+
+
+@pytest.fixture
+def el_consumer_direct_base_load(energy_usage_model_direct_load):
+    def el_consumer(el_reference_name: str, load: float):
+        return (
+            YamlElectricityConsumerBuilder()
+            .with_name(el_reference_name)
+            .with_category(ConsumerUserDefinedCategoryType.BASE_LOAD)
+            .with_energy_usage_model(energy_usage_model_direct_load(load))
+        ).validate()
+
+    return el_consumer


add _factory to all these?

jsolaas · 2024-11-25T09:15:34Z

src/libecalc/testing/yaml_builder.py

+    def get_yaml_model(
+        self,
+        frequency: Frequency,
+        resources: dict[str, MemoryResource] = None,
+    ) -> YamlModel:
+        if resources is not None:
+            resource_service = DirectResourceService(resources)
+        else:
+            resource_service = FileResourceService(working_directory=Path(""))
+
+        asset_dict = self.validate().model_dump(
+            serialize_as_any=True,
+            mode="json",
+            exclude_unset=True,
+            by_alias=True,
+        )
+
+        yaml_string = PyYamlYamlModel.dump_yaml(yaml_dict=asset_dict)
+        configuration_service = OverridableStreamConfigurationService(
+            stream=ResourceStream(name="", stream=StringIO(yaml_string)),
+        )
+        asset_yaml_model = YamlModel(
+            configuration_service=configuration_service,
+            resource_service=resource_service,
+            output_frequency=frequency,
+        ).validate_for_run()
+
+        return asset_yaml_model


I would prefer it if these builders did not know about YamlModel etc. I think we should be able to use builders to set up yaml_model, see yaml_model_factory in conftest.py.

jsolaas · 2024-11-25T09:16:21Z

tests/libecalc/presentation/exporter/test_ltp.py

+from tests.libecalc.presentation.exporter.memory_resources import (
+    generator_electricity2fuel_17MW_resource,
+    onshore_power_electricity2fuel_resource,
+    cable_loss_time_series_resource,
+    compressor_sampled_fuel_driven_resource,
+    generator_fuel_power_to_fuel_resource,
+    generator_diesel_power_to_fuel_resource,
+    max_usage_from_shore_time_series_resource,
+)


This shouldn't be necessary with fixtures

jsolaas · 2024-11-25T09:25:38Z

tests/libecalc/presentation/exporter/test_ltp.py

-                installation_name=installation_name_1,
-                genset_name="generator 1",
-                compressor_name="gas driven compressor 1",
+class TestLtp:


This looks like a test helper class. The Test prefix is used by pytest to search for tests. This class could instead be LtpTestHelper or something. I would then use __init__ and self to set values, instead of cls. We could also consider fixtures here, whatever makes the setup easier to figure out when looking at a test.

chore: use builders for ltp tests

762b144

ECALC-1813

frodehk self-assigned this Nov 22, 2024

frodehk requested a review from a team as a code owner November 22, 2024 14:07

frodehk added 3 commits November 22, 2024 15:11

chore: remove some ltp yaml factories not used

1ca0b40

ECALC-1813

chore: delete unused functions

b44d921

ECALC-1813

chore: delete installation setup not used anymore

4cc834a

ECALC-1813

jsolaas reviewed Nov 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: use builders for ltp tests #711

chore: use builders for ltp tests #711

frodehk commented Nov 22, 2024 •

edited

Loading

jsolaas Nov 25, 2024

jsolaas Nov 25, 2024

jsolaas Nov 25, 2024

jsolaas Nov 25, 2024

jsolaas Nov 25, 2024

jsolaas Nov 25, 2024

jsolaas Nov 25, 2024

jsolaas Nov 25, 2024

jsolaas Nov 25, 2024


		class OverridableStreamConfigurationService(ConfigurationService):

chore: use builders for ltp tests #711

Are you sure you want to change the base?

chore: use builders for ltp tests #711

Conversation

frodehk commented Nov 22, 2024 • edited Loading

Why is this pull request needed?

What does this pull request change?

Issues related to this change:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frodehk commented Nov 22, 2024 •

edited

Loading