Update pov_test.py

Just tried to submit a PR for this on Github.

The test results as written were inconsistent. ValueErrors in the from_pov method return “Tree could not be reoriented”. ValueErrors in the path_to method should say “No path found”.

I noticed this when trying to submit the exercise.

This specifically applies to this test case, where the tree is not being reoriented:

def test_errors_if_source_does_not_exist(self):
    tree = Tree(
        "parent",
        [
            Tree("x", [Tree("kid-0"), Tree("kid-1")]),
            Tree("sibling-0"),
            Tree("sibling-1"),
        ],
    )
    with self.assertRaises(ValueError) as err:
        tree.path_to("nonexistent", "x")
    self.assertEqual(type(err.exception), ValueError)
    self.assertEqual(err.exception.args[0], "Tree could not be reoriented")

This is just my guess but from my pov I thought this happen because the exercise expect the logic to be : re-orientating the tree first and then from that new tree pov you go and and find a path.
So if you couldn’t do the re-orientation then it would throw the error first.

I don’t believe that is the case. The last two test cases both test the path_to function without reorienting the tree, and they both expect different results even though both are supposed to raise valueErrors:

def test_errors_if_destination_does_not_exist(self):
    tree = Tree(
        "parent",
        [
            Tree("x", [Tree("kid-0"), Tree("kid-1")]),
            Tree("sibling-0"),
            Tree("sibling-1"),
        ],
    )
    with self.assertRaises(ValueError) as err:
        tree.path_to("x", "nonexistent")
    self.assertEqual(type(err.exception), ValueError)
    self.assertEqual(err.exception.args[0], "No path found")
def test_errors_if_source_does_not_exist(self):
    tree = Tree(
        "parent",
        [
            Tree("x", [Tree("kid-0"), Tree("kid-1")]),
            Tree("sibling-0"),
            Tree("sibling-1"),
        ],
    )
    with self.assertRaises(ValueError) as err:
        tree.path_to("nonexistent", "x")
    self.assertEqual(type(err.exception), ValueError)
    self.assertEqual(err.exception.args[0], "Tree could not be reoriented")
# Custom Utility Functions
def assertTreeEquals(self, result, expected):
    self.assertEqual(result, expected, "{} != {}".format(result, expected))

It is just a bug in the test case.

I suppose I missed something if it requires/expects the reorientation to build the path_to function, as I didn’t implement mine that way.

I think you are right.

I did this exercise on python quite a while ago so I forgot a lot of details.
However I did implement the same exercise for the powershell track last year, and the test suite did ask for the error to be raised about the path since the find path method was the one being called.

Let make sure and ask the maintainer about it, @BethanyG please if you can take a look.

Hi @overho79 :wave:

Welcome to the Exercism forum.

This exercise is rated as one of our most difficult and we had a bunch of trouble making the test generation template for it.

There was also some discussion around our error handling messaging and ordering, so that was removed from the instruction append.

After taking a look at the template (Here is the generated test file) and the canonical data, it appears we may be checking the wrong fields when determining the error message for that last test case.

I would welcome a fix if either you or @glaxxie would like to change the jinja2 template, regenerate the test file, and test & edit the example solution where needed (sadly, only editing the generated test file is not enough here).

Otherwise, I can add this to my list to look into. However, I have to warn you that my list is a bit long at the moment, so it might take a bit to fix. :smile:

1 Like