Skip to content

Feat/5058/add smoke tests for api happy path#3

Open
SarahJohnsonONS wants to merge 2 commits into
mainfrom
feat/5058/add-smoke-tests-for-API-happy-path
Open

Feat/5058/add smoke tests for api happy path#3
SarahJohnsonONS wants to merge 2 commits into
mainfrom
feat/5058/add-smoke-tests-for-API-happy-path

Conversation

@SarahJohnsonONS

Copy link
Copy Markdown

What

Added simple smoke tests for all Dataset API endpoints.
https://officefornationalstatistics.atlassian.net/browse/DIS-5058

Also added a behave_debug.py file, which can be used to debug behaviour tests.

How to review

Run behave and check that all tests pass. You'll need to add the dataset-editions-versions:delete permission to the list of permissions for the administrator role in dp-permissions-api/import-script/roles.json, if this hasn't been fixed yet.

I've kept the assertions very simple for now, but these can be expanded upon over time, if necessary.

Who can review

Anyone

@SarahJohnsonONS
SarahJohnsonONS requested a review from a team as a code owner July 21, 2026 13:38

@aryan-p03 aryan-p03 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are working locally which is good. Although I have some questions in general:

  • Would it be easier to have something like our Go component tests where we can do steps like "When I {{HTTP_METHOD}} {{HTTP_PATH}}"?
  • These tests include files but only the metadata is registered. If we were to go and download the file in another step then that would fail as the file does not exist. Something to think about if we were to have other test scenarios where we wanted to test file access based on user permissions.

context.response = requests.get(
f"{context.dataset_api_url}/datasets/{dataset_id}/editions/{edition_id}/versions/{version}/dimensions/{dimension}/options",
headers=context.headers,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both these function are called request_version_dimensions?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines +253 to +262
@then('the response should contain the version with ID "{version}"')
def response_should_contain_version(context, version):
data = context.response.json()
assert data["version"] == int(version)


@then('the response should contain the version with state "{state}"')
def response_should_contain_version(context, state):
data = context.response.json()
assert data["state"] == state

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both these functions are called response_should_contain_version?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Given the dataset API is available
When I request the options for dataset ID "cpih01", edition ID "time-series", version "1", dimension "geography"
Then the response status code should be "200"
And the response should contain the options for dimension "geography"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are dataset types other than static in scope for the smoke test repo? (If not, then we don't need to worry about these endpoints)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will check in standup tomorrow.

Then the response status code should be "200"
And the response should include the updated edition title "New edition title"

Scenario: Update state of a version

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the scenario line has an extra indent

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment thread features/utils.py
)
for i in range(num_versions):
_post_version(context, context.edition_id, i)
_post_file_metadata(context, i)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The random edition_id is defined at the start here but then passed into _post_version which will again ramdomise the edition_id. So does this mean each new version being posted is going to a different edition path?
So this isn't creating multiple versions within a single edition but rather creating multiple editions each with one version?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment thread behave_debug.py

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth putting something in the README about how to debug tests

Comment thread features/utils.py


def _post_file_metadata(context, version):
path = f"{context.dataset_id}/{context.edition_id}/{version}/{context.filename}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In reality the file path is {{UUID}}/filename.extension which would be more accurate to use

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants