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

Reorganize tests #1

Open
wants to merge 1 commit into
base: md_core
Choose a base branch
from

Conversation

tarasvaskiv
Copy link

No description provided.

@@ -1,4 +1,5 @@
from openprocurement.relocation.core.tests.base import test_transfer_data
from openprocurement.api.tests.base import snitch
from .transfer_blanks import change_ownership
Copy link
Member

Choose a reason for hiding this comment

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

відносні імпорти - зло)


def check_permission(self, path, token):
pass
raise NotImplementedError('Please implement check_permission method')
Copy link
Member

Choose a reason for hiding this comment

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

тут вроді і без сказання повідомлення буде цілком зрозуміло.



def simple_add_transfer(self):
data = {"access_token": "1234",
Copy link
Member

Choose a reason for hiding this comment

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

давай тут і для transfer_token впишемо більш правдоподібне значення - uuid4



def get_transfer(self):
response = self.app.get('/transfers', status=405)
Copy link
Member

Choose a reason for hiding this comment

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

завеликий відступ

self.assertEqual(response.status, '200 OK')
self.assertNotIn('transfer', response.json['data'])
self.assertNotIn('transfer_token', response.json['data'])
if 'owner' in response.json['data']:
Copy link
Member

Choose a reason for hiding this comment

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

Маєш ідеї як позбутись цієї умови?

# try to use already applied transfer
self.app.authorization = authorization
collection_path = '/'.join(self.request_path.split('/')[:-1])
if hasattr(self, 'create_token'):
Copy link
Member

Choose a reason for hiding this comment

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

ця умова зайва



def change_ownership(self):
self.prepare_ownership_change()
Copy link
Member

Choose a reason for hiding this comment

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

Це могло б бути частиною setUp-у тесту?

self.assertEqual(response.json['data']['owner'], self.first_owner)

# current owner can change his object !!!
response = self.check_permission(self.request_path, self.first_owner_token)
Copy link
Member

Choose a reason for hiding this comment

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

чому check_permission винесений окремо?

# OwnershipChangeTestMixin


def change_ownership(self):
Copy link
Member

Choose a reason for hiding this comment

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

цей тест треба розбити на декілька.
якщо деякі з операцій (створення об'єкта для тестування) будуть повторюватись, це не страшно.

@tarasvaskiv tarasvaskiv force-pushed the a563836009868071_fix_update_tests branch 2 times, most recently from 42ff972 to 0e14f42 Compare February 20, 2018 09:20
from uuid import uuid4
from copy import deepcopy

from openprocurement.relocation.core.tests.base import test_transfer_data
Copy link
Collaborator

Choose a reason for hiding this comment

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

давай тут не імпортувати test_transfer_data, а оголосити її у відповідному класі, і у тестах тягнути як self.transfer_data

@tarasvaskiv tarasvaskiv force-pushed the a563836009868071_fix_update_tests branch 2 times, most recently from 1515714 to 9a966d7 Compare February 22, 2018 09:47
@tarasvaskiv tarasvaskiv force-pushed the a563836009868071_fix_update_tests branch from 9a966d7 to 70de4da Compare February 22, 2018 17:22
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.

3 participants