My first pull request by OSerhii · Pull Request #2 · openprocurement/robot_tests.broker.dzo

OSerhii · 2016-03-22T15:48:44Z

This change is

Switched from ssh (requiring public key access) to https (allowing anonymous access)

mykhaly · 2016-07-06T15:14:36Z

👍

Reviewed 3 of 4 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 22 unresolved discussions.

dzo.robot, line 96 [r3] (raw file):

  ${download_path}=   get_download_file_path
  ${folderList_param}=   Convert To Integer   2
  ${bool_true}=   Convert To Boolean   true

${True}

dzo.robot, line 97 [r3] (raw file):

  ${folderList_param}=   Convert To Integer   2
  ${bool_true}=   Convert To Boolean   true
  ${bool_false}=   Convert To Boolean   false

${False}

dzo.robot, line 98 [r3] (raw file):

  ${bool_true}=   Convert To Boolean   true
  ${bool_false}=   Convert To Boolean   false
  ${profile}=   Evaluate   sys.modules['selenium.webdriver'].FirefoxProfile()   sys

Eval is evil. Це можна якось замінити?

dzo.robot, line 140 [r3] (raw file):

  Run Keyword And Ignore Error   Click Element   xpath=//a[@class="close icons"]
  Run Keyword If   '${mode}' == 'belowThreshold'   Створити допороговий тендер   ${tender_data}
  ...   ELSE IF   '${mode}' == 'openua' or '${mode}' == 'openeu'   Створити понадпороговий тендер   ${tender_data}

З точки зору grammar-nazi правильно було би надпороговий, а не понадпороговий. Але це не критично)

dzo.robot, line 154 [r3] (raw file):

  [return]  ${tender_uaid}

Дочекатися публікації тендера

Яким чином воно дочікується? Нема ж ніякого Wait until...

dzo.robot, line 226 [r3] (raw file):

  Input text             name=data[value][amount]          ${budget}
  Click Element          id=multiItems
  Додати предмет         ${items[0]}

Якщо буде декілька предметів, то тести на відображення предмету не пройдуть.
Раджу щось отаке:

:FOR  ${item}  in  ${tender_data.data.items}
  \  Додати предмет  ${item}

dzo.robot, line 423 [r3] (raw file):

Створити лот
  [Arguments]  ${username}  ${tender_uaid}  ${lot}   ${tender_data}=${EMPTY}
  ${lot}=   Set Variable If   '${tender_uaid}' != '0'   ${lot.data}   ${lot}

Думаю, що тут краще було би зробити перевірку на ${None}, а не 0. Ну і, відповідно, замість 0 передавати ${None}.

dzo.robot, line 796 [r3] (raw file):

  [Documentation]
  ${doc_index}=         Get Substring    ${field_name}    10   11
  Set Test Variable     ${doc_index}     ${doc_index}

Навіщо двічі ${doc_index}?

dzo.robot, line 1198 [r3] (raw file):

  Go To   ${USERS.users['${username}'].homepage}
  Wait Until Page Contains   Держзакупівлі.онлайн   10
  Пошук тедера в Мої Закупівлі   ${tender_uaid}

Пошук тедера -> Пошук теНдера

dzo.robot, line 1214 [r3] (raw file):

Модифікувати закупівлю
  [Arguments]  ${username}  ${tender_uaid}
  Пошук тедера в Мої Закупівлі   ${tender_uaid}

Пошук тедера -> Пошук теНдера

dzo.robot, line 1218 [r3] (raw file):

Пошук тедера в Мої Закупівлі

Пошук тедера -> Пошук теНдера

dzo.robot, line 1275 [r3] (raw file):

  Input text                 name=title    test_doc
  Execute Javascript         $('input[name=upload]').css({ visibility: "visible", height: "20px", width: "40px"}); $('#jAlertBack').remove();
  Choose File                name=upload     /home/username/robot_tests/src/robot_tests.broker.dzo/testFileForUpload.txt

Знаю, що це ще не готово. Пишу коментар, щоб потім про це не забути.

dzo.robot, line 1292 [r3] (raw file):

  Input text                 id=PKeyPassword                              qwerty
  Click Element              id=PKeyReadButton
  Wait Until Page Contains   Горобець                                     10

???

dzo_service.py, line 9 [r3] (raw file):

DZO_dict = {u'килограммы': u'кг',u'кілограм': u'кг', u'метри': u'м', u'пара': u'пар', u'літр': u'л', u'набір': u'наб', u'пачок': u'пач', u'послуга': u'посл', u'метри кубічні': u'м.куб', u'тони': u'т', u'метри квадратні': u'м.кв', u'кілометри': u'км', u'штуки': u'шт', u'місяць': u'міс', u'пачка': u'пачка', u'упаковка': u'уп', u'гектар': u'Га', u'лот': u'лот', u"грн": u"UAH", u"(з ПДВ)": u"True", u"(без ПДВ)": u"false", u"Код ДК 021-2015 (CPV)": u"CPV", u"Код ДК": u"ДКПП", u"ДК": u"ДКПП", u"Відкриті торги": u"aboveThresholdUA", u"Відкриті торги з публікацією англ.мовою": u"aboveThresholdEU", u"Переможець": u"active", u"місто Київ": u"м. Київ", u"НЕЦІНОВІ КРИТЕРІЇ ДО ПРЕДМЕТУ ЗАКУПІВЛІ": u"item", u"НЕЦІНОВІ КРИТЕРІЇ ДО ЛОТУ": u"lot", u"ПЕРІОД УТОЧНЕНЬ": u"active.enquiries", u"ПОДАННЯ ПРОПОЗИЦІЙ": u"active.tendering", u"ПРЕДКВАЛІФІКАЦІЯ": u"active.pre-qualification", u"АУКЦІОН": u"active.auction", u"НА РОЗГЛЯДІ": u"claim"}

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

dzo_service.py, line 30 [r3] (raw file):

def adapt_unit_names(tender_data):
    if 'unit' in tender_data['data']['items'][0]:

Не було б краще отак?

for item in tender_data.data.items:
    if 'unit' in item:
        adapt_one_item(item)

Мені здається, що доступ через крапку мав би працювати, бо ${USERS.users['${username}'].tender_data} стараємось тримати munch'ем.

dzo_service.py, line 58 [r3] (raw file):

    return tender_data

def adapt_address(tender_data):

Цей метод мені не подобається.

Чому region, а не item?
Чому хардкод?

dzo_service.py, line 73 [r3] (raw file):

def add_second_sign_after_point (amount):
    amount = str(repr(amount))

Виглядає збочено. Яка мета цього коду? Отримати з 13 типу float 13 типу string?

dzo_service.py, line 138 [r3] (raw file):

def get_lot_title(related_id, tender_data):
    for item in tender_data['data']['items']:
        if 'id' in item and item['id'] == related_id:

Здається, це можна замінити на if item.get('id') == related_id:. Але я не впевнений на 100%.

dzo_service.py, line 142 [r3] (raw file):

    for lot in tender_data['data']['lots']:
        if lot['id'] == related_id:
            lot_title = lot['title']

for lot in tender_data['data']['lots']:
        if lot['id'] == related_id:
            return lot['title'].split(':')[0]
 raise Exception

Можна вказати і якийсь конкретніший тип для Exception

dzo_service.py, line 148 [r3] (raw file):

    for item in tender_data:
        if item_id in item['description']:
            item_rel_id = item['id']

for item in tender_data:
        if item_id in item['description']:
            return item['id']
raise Exception

dzo_service.py, line 152 [r3] (raw file):

def get_field_locator(fieldname):
    field_locator = 'name=data[' + fieldname.replace('.', '][') + ']'

return 'name=data[' + fieldname.replace('.', '][') + ']'

dzo_service.py, line 155 [r3] (raw file):

    return field_locator

def get_claim_status(class_incl):

Аналогічно, як вже писав вище.

Comments from Reviewable

OSerhii · 2016-07-07T12:36:24Z

Review status: all files reviewed at latest revision, 22 unresolved discussions.

dzo.robot, line 96 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

${True}

Дякую за підказку, зміню! Підозрював що щось подібне має бути у роботі, але робилося на скору руку щоби перевірити працездатність ідеї..

dzo.robot, line 97 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

${False}

дякую, заміню.

dzo.robot, line 98 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Eval is evil. Це можна якось замінити?

Замінити не можна.Розробники Selenium2Library самі рекомендують використовувати дану конструкцію для конфігурування профайлу браузера - https://github.com/robotframework/Selenium2Library/issues/529

dzo.robot, line 140 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

З точки зору grammar-nazi правильно було би надпороговий, а не понадпороговий. Але це не критично)

Дякую за зауваження, заміню)

dzo.robot, line 154 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Яким чином воно дочікується? Нема ж ніякого Wait until...

Ця конструкція ще не дописана.. Тільки кілька днів назад дізнався що коли ви шлете нам 5хх помилки потрібно закласти у логіку драйверу очікування створення тендеру. Має бути якось так: `Дочекатися публікації тендера Reload Page Click Element xpath=//a[./text()='Редагувати'] Wait Until Element Is Visible xpath=//button[@value="save"] Click Element xpath=//button[@value="save"] Wait Until Element Is Not Visible xpath=//div[contains(text(),'Чернетка')]`

dzo.robot, line 226 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Якщо буде декілька предметів, то тести на відображення предмету не пройдуть.
Раджу щось отаке:
:FOR  ${item}  in  ${tender_data.data.items}
  \  Додати предмет  ${item}

Вірно. Писалося дуже давно для сюїтів, які лежали в окремих файлах. Буду перероблювати.

dzo.robot, line 423 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Думаю, що тут краще було би зробити перевірку на ${None}, а не 0. Ну і, відповідно, замість 0 передавати ${None}.

Згоден, напевно так буде краще, хоча на механіку ніяк не вплине. Зміню.

dzo.robot, line 796 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Навіщо двічі ${doc_index}?

Це буде перероблюватися. Так, можна було і один раз))

dzo.robot, line 1198 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Пошук тедера -> Пошук теНдера

Слушно)) Заміню... блін, як я так промахнувся..

dzo.robot, line 1214 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Пошук тедера -> Пошук теНдера

заміню)

dzo.robot, line 1218 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Пошук тедера -> Пошук теНдера

заміню)

dzo.robot, line 1275 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Знаю, що це ще не готово. Пишу коментар, щоб потім про це не забути.

ок)

dzo.robot, line 1292 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

???

Це процес накладання ЕЦП. У тестового ключа власник Горобець і після успішного накладання, на сторінці з’являється блок з текстом у якому є прізвище "Горобець".

dzo_service.py, line 9 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

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

Слушно, раніше так і було, але дофіга записів, зробив так щоби не розтягувати той словник на декілька екранів.

dzo_service.py, line 30 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Не було б краще отак?
for item in tender_data.data.items:
    if 'unit' in item:
        adapt_one_item(item)
Мені здається, що доступ через крапку мав би працювати, бо ${USERS.users['${username}'].tender_data} стараємось тримати munch'ем.

спробував, переробити через крапку.. щось не завелося. Поки залишу так, а у майбутньому порозбираюся, може перероблю.

dzo_service.py, line 58 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Цей метод мені не подобається.

Чому region, а не item?

Чому хардкод?

1) так тупанув, логічніше item - заміню. 2) схоже що це стара підпорка, думаю що взагалі видалю це

dzo_service.py, line 73 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Виглядає збочено. Яка мета цього коду? Отримати з 13 типу float 13 типу string?

старий код, перероблю. Мета функцї - у випадку якщо значення поля amount флоат з одним знаком після крапки - додати другий знак, оскільки в інтерфейсі в першому випадку зпрацьовує валідація. Це тупо, я знаю, але мене поставили перед фактом що так воно залишиться.

dzo_service.py, line 138 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Здається, це можна замінити на if item.get('id') == related_id:. Але я не впевнений на 100%.

спробую.

dzo_service.py, line 142 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

for lot in tender_data['data']['lots']:
        if lot['id'] == related_id:
            return lot['title'].split(':')[0]
 raise Exception
Можна вказати і якийсь конкретніший тип для Exception

додам.

dzo_service.py, line 148 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

for item in tender_data:
        if item_id in item['description']:
            return item['id']
raise Exception

стара функція, вже не використовується. Видалю.

dzo_service.py, line 152 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

return 'name=data[' + fieldname.replace('.', '][') + ']'

Дякую за підказку) хз чому одразу так не написав..

dzo_service.py, line 155 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Аналогічно, як вже писав вище.

додам ексепшн.

Comments from Reviewable

mykhaly · 2016-07-07T13:04:22Z

Review status: all files reviewed at latest revision, 3 unresolved discussions.

dzo.robot, line 98 [r3] (raw file):

Previously, OSerhii wrote…

Замінити не можна.Розробники Selenium2Library самі рекомендують використовувати дану конструкцію для конфігурування профайлу браузера - https://github.com/robotframework/Selenium2Library/issues/529

OK

dzo.robot, line 1292 [r3] (raw file):

Previously, OSerhii wrote…

Це процес накладання ЕЦП. У тестового ключа власник Горобець і після успішного накладання, на сторінці з’являється блок з текстом у якому є прізвище "Горобець".

OK

dzo_service.py, line 73 [r3] (raw file):
Не треба придумувати велосипед, його вже придумали:

>>> "{:.2f}".format(1)
'1.00'
>>> "{:.2f}".format(1.2)
'1.20'
>>> "{:.2f}".format(1.23)
'1.23'
>>> "{:.2f}".format(1.234)
'1.23'
>>> "{:.2f}".format(1.235)
'1.24'

Тому всю функцію можна замінити на

return "{:.2f}".format(amount)

Comments from Reviewable

OSerhii · 2016-07-07T13:09:06Z

Review status: all files reviewed at latest revision, 1 unresolved discussion.

dzo_service.py, line 73 [r3] (raw file):

Previously, mykhaly (Yurii Mykhalchuk) wrote…

Не треба придумувати велосипед, його вже придумали:
>>> "{:.2f}".format(1)
'1.00'
>>> "{:.2f}".format(1.2)
'1.20'
>>> "{:.2f}".format(1.23)
'1.23'
>>> "{:.2f}".format(1.234)
'1.23'
>>> "{:.2f}".format(1.235)
'1.24'
Тому всю функцію можна замінити на
return "{:.2f}".format(amount)

дякую за підказку)

Comments from Reviewable

mykhaly · 2016-07-13T08:40:01Z

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 6 unresolved discussions.

dzo.robot, line 97 [r4] (raw file):

Підготувати клієнт для користувача
  [Arguments]  ${username}
  Set Global Variable   ${DZO_MODIFICATION_DATE}   ${EMPTY}

Set Suite Variable краще

dzo.robot, line 273 [r4] (raw file):

  [Arguments]   ${feature}  ${feature_index}  ${tender_data}=${EMPTY}  ${relatedItem}=${EMPTY}
  ${enum_length}=   Get Length   ${feature.enum}
  ${relatedItem}=   Run Keyword If   "${feature.featureOf}" == "item" and '${relatedItem}' == '${EMPTY}'

Здається, тут не треба порівнювати стрінги. Краще буде ${relatedItem} == ${Empty}

dzo.robot, line 439 [r4] (raw file):

Створити лот
  [Arguments]  ${username}  ${tender_uaid}  ${lot}   ${tender_data}=${EMPTY}
  ${lot}=   Set Variable If   '${tender_uaid}' != '${None}'   ${lot.data}   ${lot}

Тут теж краще порівняти ${tender_uaid} != ${None}

dzo.robot, line 445 [r4] (raw file):

  ${lot_index}=   Evaluate   int($lot_index) - 1
  Execute Javascript     $(".topFixed").remove();
  Run Keyword If   '${tender_uaid}' != '${None}'   Run Keywords

Так, як вище.

dzo.robot, line 727 [r4] (raw file):

  ${status}=   Run Keyword And Return Status   Should Not Be Equal   ${DZO_MODIFICATION_DATE}   ${last_mod_date}
  Run Keyword If   ${status}   dzo.Пошук тендера по ідентифікатору   ${username}   ${tender_uaid}
  Set Global Variable   ${DZO_MODIFICATION_DATE}   ${last_mod_date}

Set Suite Variable

dzo.robot, line 731 [r4] (raw file):

Отримати інформацію із предмету
  [Arguments]  ${username}  ${tender_uaid}  ${item_id}  ${field_name}
  Пошук тендера у разі наявності змін   ${TENDER['LAST_MODIFICATION_DATE']}   ${username}   ${tender_uaid}

Якщо це працює, то наступний рядок з Пошук тендера по ідентифікатору варто забрати, мабуть. І всюди по коду нижче.

Comments from Reviewable

OSerhii pushed a commit to OSerhii/robot_tests.broker.dzo that referenced this pull request May 5, 2016

Merge pull request openprocurement#2 from yankee7/master

3a5d9f4

Switched from ssh (requiring public key access) to https (allowing anonymous access)

OSerhii force-pushed the master branch from 58801e0 to b0f5376 Compare July 6, 2016 11:16

OSerhii added 2 commits December 15, 2019 14:39

Add kwds for Planning

454301e

WIP

9e984dd

OSerhii force-pushed the master branch from c1a0676 to 9e984dd Compare December 27, 2019 09:43

Delete old key

2fe5ea2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

My first pull request#2

My first pull request#2
OSerhii wants to merge 3 commits intoopenprocurement:masterfrom
OSerhii:master

OSerhii commented Mar 22, 2016

Uh oh!

mykhaly commented Jul 6, 2016

Uh oh!

OSerhii commented Jul 7, 2016

Uh oh!

mykhaly commented Jul 7, 2016

Uh oh!

OSerhii commented Jul 7, 2016

Uh oh!

mykhaly commented Jul 13, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

OSerhii commented Mar 22, 2016

Uh oh!

mykhaly commented Jul 6, 2016

Uh oh!

OSerhii commented Jul 7, 2016

Uh oh!

mykhaly commented Jul 7, 2016

Uh oh!

OSerhii commented Jul 7, 2016

Uh oh!

mykhaly commented Jul 13, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants