Skip to content

Add tests for homework_1#4

Open
SashoStoichkov wants to merge 4 commits intod0ivanov:masterfrom
SashoStoichkov:master
Open

Add tests for homework_1#4
SashoStoichkov wants to merge 4 commits intod0ivanov:masterfrom
SashoStoichkov:master

Conversation

@SashoStoichkov
Copy link
Copy Markdown

No description provided.


from task2 import has_same_ingredients
from task2 import isStronger, leastStronger, strongRelation

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

По условие решението на задачите трябва да се пише в solution.py файл. Идеята на това да има конкретни инструкции за предаване на решения е да можем, сваляйки чието и да е решение директно да можем да изпълним тестовете върху това решение. В тестовете, които ти си добавил си import-вал функции от модули, които ти си си дефинирал, но твоите съученици не са длъжни да имат същите модули (би трябвало да имат само solution.py) и когато за чуждо решение се опитаме да изпълним тези тестовете, те ще гръмнат заради липсата на модулите task1, task2 etc.

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.

Сгрешил съм файла, съжалявам - сега ще кача правилния

self.assertEqual(true_result, expected_result)

class TestHasSameIngredients(unittest.TestCase):
def test_function_when_result_must_be_true(self):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Подобно на предния ми коментар тук ползваш някаква твоя helper функция предполагам, чрез която да решиш някоя задача. Такава функция не се изисква в условието, съответно твоите съученици не са длъжни да са се сетили да използват точно такава helper функция.

@SashoStoichkov
Copy link
Copy Markdown
Author

Сега трябва да са правилните :)

@@ -0,0 +1,111 @@
from solution import group_by_f
from solution import isStronger, leastStronger, strongRelation
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

snake case навсякъде. Там където не е, е наша грешка - лош навик от Java. За completeness можеш да го фикснеш и в sample_test.py

Copy link
Copy Markdown
Owner

@d0ivanov d0ivanov left a comment

Choose a reason for hiding this comment

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

LGTM. Ship it след като адресираш коментара


expected_result = [(('medicine1', [('p', 5), ('q', 3)]), []), (('medicine2', [('p', 4), ('q', 3)]), ['medicine1']), (('medicine3', [('p', 3)]), ['medicine1', 'medicine2'])]

self.assertEqual(strongRelation([medicine1, medicine2, medicine3]), expected_result)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

плс фиксни го и в sample_test.py

C = ("C", [("p", 3)])
self.assertEqual(solution.strong_relation([A, B, C]), [
(A, []), (B, ['A']), (C, ['A', 'B'])])
(("A", [("p", 5), ("q", 3)]), []),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Междудругото така както си го написал работи да, но и преди твоите промени тестът си е бил верен. Тези А, B, C реферират променливите, декларирани малко по-нагоре, а не са string-ове ( имена на лекарства). Не успях по-рано това да го забележа в snippet-a, който беше цитирал в issue-то си, но като цяло по-добре да си използваме вече дефинираните променливи, от колкото да преписваме нещо, което вече имаме.

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.

Аааа ок. Моя грешка тогава - съжалявам :(

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