Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new acceptance test data loader #1051

Merged
merged 4 commits into from
May 24, 2024
Merged

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented May 23, 2024

https://eaflood.atlassian.net/browse/WATER-3981

The water-abstraction-service has a mechanism for seeding data for testing that relies on loading in YAML fixture files, sort of. Some content comes from these, but other stuff is hard-coded into the 'loaders'.

The legacy solution depends heavily on using placeholders to define references between the entities. The benefit is that you can add things in any order, and it's easier to see x links to y. The downside is that the loaders are very complex as they try to ensure the records are created in the right order, and the right placeholders in the YAML fixtures are updated. You also have to 'know' how they work because they can be inconsistent. For example, sometimes it will handle setting a lookup value, other times you have to create a 'test' lookup value and then reference it. This has led to occasions where a fixture is trying to load in a lookup value that already exists.

Finally, there is some undocumented functionality where certain placeholders are not references but instructions to generate dynamic data. All together, it is more than 7,000 lines of code, including 5,000 lines of YAML and 1,500 lines of JavaScript. To top it off, it often falls over for no discernible reason.

Long story short, we're going to replace it with something simpler yet more flexible!

Our new solution is going to be based on the hard work we've already invested in making our test helpers. We use these to quickly create meaningful records in the DB which we can then write unit tests against. So, why not reuse them to do the same for our acceptance tests!?

This change adds a new endpoint; /data/load. Our water-abstraction-acceptance-tests will send a JSON payload that defines what entities it wants created and with what properties.

{
  "regions": [
    {
      "id": "d0a4123d-1e19-480d-9dd4-f70f3387c4b9",
      "chargeRegionId": "S",
      "naldRegionId": 9,
      "displayName": "Test Region",
      "name": "Test Region"
    }
  ],
  "licences": [
    {
      "id": "f8702a6a-f61d-4b0a-9af3-9a53768ee516",
      "licenceRef": "AT/TEST/01",
      "regionId": "d0a4123d-1e19-480d-9dd4-f70f3387c4b9",
      "regions": {
        "historicalAreaCode": "SAAR",
        "regionalChargeArea": "Southern"
      },
      "startDate": "2022-04-01",
      "waterUndertaker": true
    }
  ],
  "licenceDocumentHeaders": [
    {
      "id": "282b226e-c47b-4dcc-bbb0-94648fb6b242",
      "regimeEntityId": {
        "schema": "crm",
        "table": "entity",
        "lookup": "entityType",
        "value": "regime",
        "select": "entityId"
      }
    }
  ],
}

As you can see we've eschewed clever referencing. Instead, when you create the fixture in water-abstraction-acceptance-tests you specify the ID's directly. For this to work, the order of the entities matter. The loader we are adding will process the JSON payload from top to bottom. For example, the region in this payload will be created first. Then when the licence is created its foreign key constraint won't error because the region it is referencing exists.

Obviously, with defined ID's you cannot load the same fixture without first having called /data/tear-down to delete any existing test data. But this is the same requirement for the legacy solution.

One problem that needed to be overcome was getting the ID's for lookup values. For example, we need to load a charge reference that is linked to the charge category 4.2.1. The categories exist in a lookup table, and the charge references table has a foreign key constraint. So, we must link to something that exists in that table. The problem is the ID's are generated when the table is created and populated as part of the migrations. That means they will be different across all environments. For this situation, rather than setting a value you can set an object.

  "regimeEntityId": {
    "schema": "crm",
    "table": "entity",
    "lookup": "entityType",
    "value": "regime",
    "select": "entityId"
  }

This is the one 'clever' thing our loader does. It will use the information in the object to query the DB and extract the value requested. This will then be used when the entity is passed to the helper's .add() method.

That's it. Essentially we're doing the same thing we do in our unit tests. Passing values to our helpers and letting them deal with creating the records. Only the request is coming via a web request.

https://eaflood.atlassian.net/browse/WATER-3981

https://eaflood.atlassian.net/browse/WATER-3981

The [water-abstraction-service](https://gitub.com/DEFRA/water-abstraction-service) has a mechanism for seeding data for testing that relies on loading in YAML fixture files, sort of. Some content comes from these, but other stuff is hard-coded into the 'loaders'.

The legacy solution depends heavily on using placeholders to define references between the entities. The benefit is you can add things in any order and it's easier to see `x` links to `y`. The downside is the loaders are very complex as they try to ensure the records are created in the right order, and the right placeholders in the YAML fixtures are updated. You also have to 'know' how they work because they can be inconsistent. For example, sometimes it will handle setting a lookup value, other times you have to create a 'test' lookup value and then reference it. This has led to occasions where a fixture is trying to load in a lookup value that already exists.

Finally, there is some undocumented functionality where certain placeholders are not references but instructions to generate dynamic data. All together, it is more than 7,000 lines of code including 5,000 lines of YAML and 1,500 lines of JavaScript. To top it off, it often falls over for no discernable reason.

Long story short, we're going to replace it with something simpler yet more flexible!

Our new solution is going to be based on the hard work we've already invested in making our test helpers. We use these to quickly create meaningful records in the DB which we can then write unit tests against. So, why not reuse them to do the same for our acceptance tests!?

This change adds a new endpoint; `/data/load`. Our [water-abstraction-acceptance-tests](https://github.com/DEFRA/water-abstraction-acceptance-tests) will send a JSON payload that defines what entities it wants created and with what properties.

```json
{
  "regions": [
    {
      "id": "d0a4123d-1e19-480d-9dd4-f70f3387c4b9",
      "chargeRegionId": "S",
      "naldRegionId": 9,
      "displayName": "Test Region",
      "name": "Test Region"
    }
  ],
  "licences": [
    {
      "id": "f8702a6a-f61d-4b0a-9af3-9a53768ee516",
      "licenceRef": "AT/TEST/01",
      "regionId": "d0a4123d-1e19-480d-9dd4-f70f3387c4b9",
      "regions": {
        "historicalAreaCode": "SAAR",
        "regionalChargeArea": "Southern"
      },
      "startDate": "2022-04-01",
      "waterUndertaker": true
    }
  ],
  "licenceDocumentHeaders": [
    {
      "id": "282b226e-c47b-4dcc-bbb0-94648fb6b242",
      "regimeEntityId": {
        "schema": "crm",
        "table": "entity",
        "lookup": "entityType",
        "value": "regime",
        "select": "entityId"
      }
    }
  ],
}
```

As you can see we've eschewed clever referencing. Instead, when you create the fixture in **water-abstraction-acceptance-tests** you specify the ID's directly. For this to work, the order of the entities matter. The loader we are adding will process the JSON payload from top to bottom. For example, the region in this payload will be created first. Then when the licence is created its foreign key constraint won't error because the region it is referencing exists.

Obviously, with defined ID's you cannot load the same fixture without first having called `/data/tear-down` to delete any existing test data. But this is the same requirement for the legacy solution.

One problem that needed to be overcome was getting the ID's for lookup values. For example, we need to load a charge reference that is linked to the charge category `4.2.1`. The categories exist in a lookup table, and the charge references table has a foreign key constraint. So, we _must_ link to something that exists in that table. The problem is the ID's are generated when the table is created and populated as part of the migrations. That means they will be different across all environments. For this situation, rather than setting a value you can set an object.

```json
  "regimeEntityId": {
    "schema": "crm",
    "table": "entity",
    "lookup": "entityType",
    "value": "regime",
    "select": "entityId"
  }
```

This is the one 'clever' thing our loader does. It will use the information in the object to query the DB and extract the value requested. This will then be used when the entity is passed to the helper's `.add()` method.

That's it. Essentially we're doing the same thing we do in our unit tests. Passing values to our helpers and letting them deal with creating the records. Only the request is coming via a web request.
@Cruikshanks Cruikshanks added the enhancement New feature or request label May 23, 2024
@Cruikshanks Cruikshanks self-assigned this May 23, 2024
@Cruikshanks Cruikshanks marked this pull request as ready for review May 24, 2024 13:35
@Cruikshanks Cruikshanks merged commit 9b240f8 into main May 24, 2024
6 checks passed
@Cruikshanks Cruikshanks deleted the add-acceptance-test-data-loader branch May 24, 2024 13:35
Cruikshanks added a commit to DEFRA/water-abstraction-acceptance-tests that referenced this pull request May 24, 2024
https://eaflood.atlassian.net/browse/WATER-3981

The [water-abstraction-service](https://gitub.com/DEFRA/water-abstraction-service) has a mechanism for seeding data for testing that relies on loading in YAML fixture files, sort of. Some content comes from these, but other stuff is hard-coded into the 'loaders'.

This is connected to an endpoint that we currently call to 'setup' test data based on the fixture specified. However, it is slow and prone to fail. It is also horrendous to try and add new 'fixtures' to because of its complexity.

So, we have built an alternative [test data loader](DEFRA/water-abstraction-system#1051). This will rely on Cypress fixture files we define in this project.

In this change we are adding a custom Cypress command we can use to send fixtures we'll create in forthcoming PRs.
Cruikshanks added a commit to DEFRA/water-abstraction-acceptance-tests that referenced this pull request May 24, 2024
https://eaflood.atlassian.net/browse/WATER-3981

The [water-abstraction-service](https://gitub.com/DEFRA/water-abstraction-service) has a mechanism for seeding data for testing that relies on loading in YAML fixture files, sort of. Some content comes from these, but other stuff is hard-coded into the 'loaders'.

This is connected to an endpoint that we currently call to 'setup' test data based on the fixture specified. However, it is slow and prone to failure. It is also horrendous to try and add new 'fixtures' because of its complexity.

So, we have built an alternative [test data loader](DEFRA/water-abstraction-system#1051). This will rely on Cypress fixture files we define in this project.

In this change, we are adding a custom Cypress command we can use to send fixtures we'll create in forthcoming PRs.
Cruikshanks added a commit to DEFRA/water-abstraction-acceptance-tests that referenced this pull request May 24, 2024
https://eaflood.atlassian.net/browse/WATER-3981

The [water-abstraction-service](https://gitub.com/DEFRA/water-abstraction-service) has a mechanism for seeding data for testing that relies on loading in YAML fixture files, sort of. Some content comes from these, but other stuff is hard-coded into the 'loaders'.

The legacy solution depends heavily on using placeholders to define references between the entities. The benefit is that you can add things in any order, and it's easier to see `x` links to `y`. The downside is that the loaders are very complex as they try to ensure the records are created in the right order, and the right placeholders in the YAML fixtures are updated. You also have to 'know' how they work because they can be inconsistent. For example, sometimes it will handle setting a lookup value, other times you have to create a 'test' lookup value and then reference it. This has led to occasions where a fixture is trying to load in a lookup value that already exists.

Finally, there is some undocumented functionality where certain placeholders are not references but instructions to generate dynamic data. All together, it is more than 7,000 lines of code, including 5,000 lines of YAML and 1,500 lines of JavaScript. To top it off, it often falls over for no discernible reason.

Long story short, we're going to replace it with something simpler yet more flexible!

Our new solution is going to be based on the hard work we've already invested in making our test helpers. We use these to quickly create meaningful records in the DB which we can then write unit tests against. So, why not reuse them to do the same for our acceptance tests!?

We made that change in [Add new acceptance test data loader](DEFRA/water-abstraction-system#1051). This change adds the first new fixture and updates those tests that were using SROC billing data to use it

We load the fixture and then send it as a JSON payload that defines what entities we want created and with what properties.

```json
{
  "regions": [
    {
      "id": "d0a4123d-1e19-480d-9dd4-f70f3387c4b9",
      "chargeRegionId": "S",
      "naldRegionId": 9,
      "displayName": "Test Region",
      "name": "Test Region"
    }
  ],
  "licences": [
    {
      "id": "f8702a6a-f61d-4b0a-9af3-9a53768ee516",
      "licenceRef": "AT/TEST/01",
      "regionId": "d0a4123d-1e19-480d-9dd4-f70f3387c4b9",
      "regions": {
        "historicalAreaCode": "SAAR",
        "regionalChargeArea": "Southern"
      },
      "startDate": "2022-04-01",
      "waterUndertaker": true
    }
  ],
  "licenceDocumentHeaders": [
    {
      "id": "282b226e-c47b-4dcc-bbb0-94648fb6b242",
      "regimeEntityId": {
        "schema": "crm",
        "table": "entity",
        "lookup": "entityType",
        "value": "regime",
        "select": "entityId"
      }
    }
  ],
}
```

As you can see we've eschewed clever referencing. Instead, when we create the fixture we specify the ID's directly. For this to work, the order of the entities matter. The loader we are added will process the JSON payload from top to bottom. For example, the region in this payload will be created first. Then when the licence is created its foreign key constraint won't error because the region it is referencing exists.

Obviously, with defined ID's you cannot load the same fixture without first having called `/data/tear-down` to delete any existing test data. But this is the same requirement for the legacy solution.

One problem that needed to be overcome was getting the ID's for lookup values. For example, we need to load a charge reference that is linked to the charge category `4.2.1`. The categories exist in a lookup table, and the charge references table has a foreign key constraint. So, we _must_ link to something that exists in that table. The problem is the ID's are generated when the table is created and populated as part of the migrations. That means they will be different across all environments. For this situation, rather than setting a value you can set an object.

```json
  "regimeEntityId": {
    "schema": "crm",
    "table": "entity",
    "lookup": "entityType",
    "value": "regime",
    "select": "entityId"
  }
```

This is the one 'clever' thing our loader does. It will use the information in the object to query the DB and extract the value requested. This will then be used when the entity is passed to the helper's `.add()` method.
Cruikshanks added a commit to DEFRA/water-abstraction-acceptance-tests that referenced this pull request May 24, 2024
https://eaflood.atlassian.net/browse/WATER-3981

The [water-abstraction-service](https://gitub.com/DEFRA/water-abstraction-service) has a mechanism for seeding data for testing that relies on loading in YAML fixture files, sort of. Some content comes from these, but other stuff is hard-coded into the 'loaders'.

The legacy solution depends heavily on using placeholders to define references between the entities. The benefit is that you can add things in any order, and it's easier to see `x` links to `y`. The downside is that the loaders are very complex as they try to ensure the records are created in the right order, and the right placeholders in the YAML fixtures are updated. You also have to 'know' how they work because they can be inconsistent. For example, sometimes it will handle setting a lookup value, other times you have to create a 'test' lookup value and then reference it. This has led to occasions where a fixture is trying to load in a lookup value that already exists.

Finally, there is some undocumented functionality where certain placeholders are not references but instructions to generate dynamic data. All together, it is more than 7,000 lines of code, including 5,000 lines of YAML and 1,500 lines of JavaScript. To top it off, it often falls over for no discernible reason.

Long story short, we're going to replace it with something simpler yet more flexible!

Our new solution is going to be based on the hard work we've already invested in making our test helpers. We use these to quickly create meaningful records in the DB which we can then write unit tests against. So, why not reuse them to do the same for our acceptance tests!?

We made that change in [Add new acceptance test data loader](DEFRA/water-abstraction-system#1051). This change adds the first new fixture and updates those tests that were using SROC billing data to use it

We load the fixture and then send it as a JSON payload that defines what entities we want created and with what properties.

```json
{
  "regions": [
    {
      "id": "d0a4123d-1e19-480d-9dd4-f70f3387c4b9",
      "chargeRegionId": "S",
      "naldRegionId": 9,
      "displayName": "Test Region",
      "name": "Test Region"
    }
  ],
  "licences": [
    {
      "id": "f8702a6a-f61d-4b0a-9af3-9a53768ee516",
      "licenceRef": "AT/TEST/01",
      "regionId": "d0a4123d-1e19-480d-9dd4-f70f3387c4b9",
      "regions": {
        "historicalAreaCode": "SAAR",
        "regionalChargeArea": "Southern"
      },
      "startDate": "2022-04-01",
      "waterUndertaker": true
    }
  ],
  "licenceDocumentHeaders": [
    {
      "id": "282b226e-c47b-4dcc-bbb0-94648fb6b242",
      "regimeEntityId": {
        "schema": "crm",
        "table": "entity",
        "lookup": "entityType",
        "value": "regime",
        "select": "entityId"
      }
    }
  ],
}
```

As you can see, we've eschewed clever referencing. Instead, when we create the fixture we specify the ID's directly. For this to work, the order of the entities matter. The loader we are added will process the JSON payload from top to bottom. For example, the region in this payload will be created first. Then when the licence is created its foreign key constraint won't error because the region it is referencing exists.

Obviously, with defined ID's you cannot load the same fixture without first having called `/data/tear-down` to delete any existing test data. But this is the same requirement for the legacy solution.

One problem that needed to be overcome was getting the ID's for lookup values. For example, we need to load a charge reference that is linked to the charge category `4.2.1`. The categories exist in a lookup table, and the charge references table has a foreign key constraint. So, we _must_ link to something that exists in that table. The problem is the ID's are generated when the table is created and populated as part of the migrations. That means they will be different across all environments. For this situation, rather than setting a value, you can set an object.

```json
  "regimeEntityId": {
    "schema": "crm",
    "table": "entity",
    "lookup": "entityType",
    "value": "regime",
    "select": "entityId"
  }
```

This is the one 'clever' thing our loader does. It will use the information in the object to query the DB and extract the value requested. This will then be used when the entity is passed to the helper's `.add()` method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant