Skip to content

Conversation

@polonskyilya
Copy link
Collaborator

Hi,
Terribly sorry for the wrong pull requests I've made a week ago. I believe that this one is correct.
Thank you in advance for the patience :)

all except cipher
# your code here
pass
def test_None(self):
self.assertTrue(questions.is_unique_string(None))
Copy link
Owner

Choose a reason for hiding this comment

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

is_unique_string expects string as input, not None

def test_if_return_string(self):
self.assertIsInstance(questions.reverse_words_concatenation(['x', 'y', 'z']), str)

def tes_str(self):
Copy link
Owner

Choose a reason for hiding this comment

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

function name must starts with test_

def test_not_unique(self):
self.assertFalse(questions.is_unique_string('aabbcc'))

def tes_unique(self):
Copy link
Owner

Choose a reason for hiding this comment

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

function name must starts with test_

self.assertFalse(questions.prime_number(0))

def test_none(self):
self.assertFalse(questions.prime_number(None))
Copy link
Owner

Choose a reason for hiding this comment

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

prime_number should receive only int as an argument

self.assertFalse(questions.prime_number(None))

def test_float(self):
self.assertFalse(questions.prime_number(5.5))
Copy link
Owner

Choose a reason for hiding this comment

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

prime_number should receive only int as an argument

def test_sample(self):
# your code here
pass
def negative_vaules(self):
Copy link
Owner

Choose a reason for hiding this comment

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

Test function must starts with test_

def negative_vaules(self):
men = {"John": -20, "Abraham": -45}
women = {"July": -18, "Kim": -26}
result = ('John', 'July')
Copy link
Owner

Choose a reason for hiding this comment

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

aren't ('July', 'John') a valid result as well?

result = ('John', 'July')
self.assertEqual(questions.pair_match(men, women), result)

def is_result_tuple(self):
Copy link
Owner

Choose a reason for hiding this comment

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

Test function must starts with test_

women = {"July": -18, "Kim": -26}
self.assertIsInstance(questions.pair_match(men,women), tuple)

def none_value(self):
Copy link
Owner

Choose a reason for hiding this comment

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

Test function must starts with test_

women = {"July": None, "Kim": -26}
self.assertEqual(questions.pair_match(men,women), ('Abraham', 'Kim'))

def none_key(self):
Copy link
Owner

Choose a reason for hiding this comment

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

Test function must starts with test_

women = {"July": 55, "Kim": 256}
self.assertEqual(questions.pair_match(men,women), (None, 'Kim'))

def zero_values(self):
Copy link
Owner

Choose a reason for hiding this comment

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

Test function must starts with test_

self.assertEqual(questions.bad_average(20, 40, 40), 33)

def test_float(self):
self.assertEqual(questions.bad_average(5.5, 5.5, 5.5), 5)
Copy link
Owner

Choose a reason for hiding this comment

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

The answer should be 5.5

self.assertEqual(questions.bad_average(5.5, 5.5, 5.5), 5)

def test_return_int(self):
self.assertIsInstance(questions.bad_average(0, 0, 0), int)
Copy link
Owner

Choose a reason for hiding this comment

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

float is a valid response type as well

data = {
"Ben": 78,
"Dan": 88,
"Nathan": None,
Copy link
Owner

@alonitac alonitac Oct 13, 2022

Choose a reason for hiding this comment

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

The function expects dict of str -> int | float. So no need to test this case


def test_value_string(self):
data = {
None: 'x',
Copy link
Owner

Choose a reason for hiding this comment

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

The function expects dict of str -> int | float. So no need to test this case

}
self.assertEqual(questions.best_student(data), 'Tal')

def test_key_not_str(self):
Copy link
Owner

Choose a reason for hiding this comment

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

The function expects dict of str -> int | float. So no need to test this case

self.assertEqual(questions.best_student(data), 'Tal')


class TestPrintDictAsTable(unittest.TestCase):
Copy link
Owner

Choose a reason for hiding this comment

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

We will remove this function from the katas

def test_sample(self):
# your code here
pass
def test_negative(self):
Copy link
Owner

Choose a reason for hiding this comment

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

n should be a natural number (positive integer)

n = '7x'
self.assertEqual(questions.seven_boom(n), [])

class TestCaesarCipher(unittest.TestCase):
Copy link
Owner

Choose a reason for hiding this comment

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

This test suite should test caesar_cipher, not sum_of_digits

Copy link
Owner

@alonitac alonitac left a comment

Choose a reason for hiding this comment

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

Hi Ilya, great job! I left some comments

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