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

feat: Add integration tests for storages, proxy configuration #49

Merged
merged 10 commits into from
Feb 8, 2023

Conversation

jirimoravcik
Copy link
Member

I am open to suggestions for more tests or some more complicated cases.

We should also think if running the/same similar code in unit and integration tests has some value or not.

@github-actions github-actions bot added this to the 56th sprint - Platform team milestone Feb 6, 2023
@github-actions github-actions bot added the t-platform Issues with this label are in the ownership of the platform team. label Feb 6, 2023
assert list_page.items[-1]['id'] == desired_item_count - 1


class TestActorOpenDataset:
Copy link
Member

Choose a reason for hiding this comment

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

I will test if open works for the dataset name and as you pass ID, we probably didn't test this in unit tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

from .conftest import ActorFactory


class TestActorOpenKeyValueStore:
Copy link
Member

Choose a reason for hiding this comment

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

Same here, I would test with name and ID param.

from .conftest import ActorFactory


class TestActorOpenRequestQueue:
Copy link
Member

Choose a reason for hiding this comment

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

Same here, I would test with name and ID param.

Copy link
Member

@fnesveda fnesveda left a comment

Choose a reason for hiding this comment

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

Cool! I had a few comments, plus I agree with Kuba's comments about testing the named storages.

tests/integration/test_actor_create_proxy_configuration.py Outdated Show resolved Hide resolved
tests/integration/test_actor_dataset.py Outdated Show resolved Hide resolved
tests/integration/test_actor_dataset.py Outdated Show resolved Hide resolved
Comment on lines +10 to +11
rq1 = await Actor.open_request_queue()
rq2 = await Actor.open_request_queue()
Copy link
Member

Choose a reason for hiding this comment

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

We should also check the RequestQueue class functionality a bit, so that we know it works on the platform.

@jirimoravcik
Copy link
Member Author

So:

  • Fixed the newlines that were not fixable by make format
  • Added secret input to the get input test
  • Improved push_data test
  • Created tests for named storages.
  • Added a test for the request queue (enqueue 100 urls, fetch them and mark them as handled)

I found out some problems with datetimes, specifically TypeError: can't subtract offset-naive and offset-aware datetimes when diffing in RequestQueue... So I patched it in a way that it works both locally and on platform, we should unify it later.

@jirimoravcik jirimoravcik merged commit fd0566e into master Feb 8, 2023
@jirimoravcik jirimoravcik deleted the feature/storage-proxy-conf-int-tests branch February 8, 2023 16:19
@fnesveda fnesveda added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-platform Issues with this label are in the ownership of the platform team. validated Issues that are resolved and their solutions fulfill the acceptance criteria.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants