test_link.py: Add more tests and cleanup

[schwehr]

• Drop RANDOM_BBOX and RANDOM_GEOM as they are not important to the
  methods under test
• Just import datetime. Try to stick to importing just modules.
• Create an item in setUp
• Add test_minimal with the simplest instance creation call
• Add a limited test_relative

#155

[codecov-io]

Codecov Report

Merging #211 into develop will increase coverage by 0.29%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #211      +/-   ##
===========================================
+ Coverage    92.32%   92.62%   +0.29%     
===========================================
  Files           28       28              
  Lines         3349     3375      +26     
===========================================
+ Hits          3092     3126      +34     
+ Misses         257      249       -8     

Impacted Files	Coverage Δ
pystac/extensions/version.py	98.26% <0.00%> (ø)
pystac/utils.py	97.70% <0.00%> (+0.05%)	⬆️
pystac/stac_object.py	95.04% <0.00%> (+0.15%)	⬆️
pystac/catalog.py	90.27% <0.00%> (+0.34%)	⬆️
pystac/link.py	98.24% <0.00%> (+0.97%)	⬆️
pystac/cache.py	100.00% <0.00%> (+6.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fcfb44...df45327. Read the comment docs.

[schwehr]

Looking at the coverage this way:

coverage run --source=pystac/ -m unittest discover tests/  -v -k test_link
coverage report -m | grep link.py

I was able to get the missing coverage down to:

coverage report -m | egrep 'Name|link.py'
Name                                        Stmts   Miss  Cover   Missing
pystac/link.py                                110     12    89%   109, 129, 146-161

With all tests and these changes, these are the only missing lines:

coverage run --source=pystac/ -m unittest discover tests/
coverage report -m | egrep 'Name|link.py'
Name                                        Stmts   Miss  Cover   Missing
pystac/link.py                                110      2    98%   151, 155

Should I add these tests to this pr?

    # TODO: Test get_href when href is absolute and there is an owner
    # TODO: Test get_absolute_href when there is an owner
    # TODO: Test when resolve_stac_object on link when target is a str.

    def test_resolve_stac_object_no_root_and_target_is_item(self):
        link = pystac.Link('my rel', target=self.item)
        link.resolve_stac_object()


class StaticLinkTest(unittest.TestCase):
    def test_from_dict_round_trip(self):
        test_cases = [
            {'rel': '', 'href': ''},  # Not valid, but works.
            {'rel': 'r', 'href': 't'},
            {'rel': 'r', 'href': '/t'},
            {'rel': 'r', 'href': 't', 'type': 'a/b', 'title': 't', 'c': 'd', 1: 2},
            {'rel': 'self', 'href': 't'},  # Special case.
        ]
        for d in test_cases:
            d2 = pystac.Link.from_dict(d).to_dict()
            self.assertEqual(d, d2)

    def test_from_dict_link_type(self):
        test_cases = [
            ({'rel': '', 'href': 'https://a'}, pystac.LinkType.ABSOLUTE),
            ({'rel': '', 'href': '/a'}, pystac.LinkType.ABSOLUTE),
            ({'rel': '', 'href': 'a'}, pystac.LinkType.RELATIVE),
            ({'rel': '', 'href': './a'}, pystac.LinkType.RELATIVE),
            # 'self' is a special case.
            ({'rel': 'self', 'href': 'does not matter'}, pystac.LinkType.ABSOLUTE),
        ]
        for case in test_cases:
            item = pystac.Link.from_dict(case[0])
            self.assertEqual(case[1], item.link_type)

    def test_from_dict_failures(self):
        for d in [{}, {'href': 't'}, {'rel': 'r'}]:
            with self.assertRaises(KeyError):
                pystac.Link.from_dict(d)

        for d in [{'rel': '', 'href': 1}, {'rel': '', 'href': None},]:
            with self.assertRaises(AttributeError):
                pystac.Link.from_dict(d)

    def test_collection(self):
        c = pystac.Collection('collection id', 'desc', extent=None)
        link = pystac.Link.collection(c)
        expected = {'rel': 'collection', 'href': None, 'type': 'application/json'}
        self.assertEqual(expected, link.to_dict())

    def test_child(self):
        c = pystac.Collection('collection id', 'desc', extent=None)
        link = pystac.Link.child(c)
        expected = {'rel': 'child', 'href': None, 'type': 'application/json'}
        self.assertEqual(expected, link.to_dict())

This boils now to how important it is to have unittests for the module cover the module. I appreciate tests that get right to the heart of trouble when things break, but it is a bit more testing code to have around.

Head is currently showing:

coverage run --source=pystac/ -m unittest discover tests/
coverage report -m | egrep 'Name|link.py'
Name                                        Stmts   Miss  Cover   Missing
pystac/link.py                                110      3    97%   134, 151, 155

coverage run --source=pystac/ -m unittest discover tests/ -v -k test_link
test_link_does_not_fail_if_href_is_none (test_link.LinkTest)
Test to ensure get_href does not fail when the href is None ... ok

coverage report -m | egrep 'Name|link.py'
Name                                        Stmts   Miss  Cover   Missing
pystac/link.py                                110     48    56%   86-87, 106, 109-111, 123-131, 134, 146-161, 169, 191-205, 217, 233-248, 268, 278

[schwehr]

Went ahead with cleaning up and adding the rest of the tests that I wrote.

[lossyrob]

There are a few TODOs in the code; based on your last comment it seems like these might be left over?

In general I think we should prefer Issues to code TODOs.

Also, if there's TODO's left for the PR, it's best to put the PR in the "Draft" state. Sometimes I'll put a checklist of TODOs in the PR description that I can check off as I accomplish tasks, and after I'm ready to move the PR out of Draft phase, remove the TODO list from the description.

[schwehr]

There are a few TODOs in the code; based on your last comment it seems like these might be left over?

In general I think we should prefer Issues to code TODOs.

Also, if there's TODO's left for the PR, it's best to put the PR in the "Draft" state. Sometimes I'll put a checklist of TODOs in the PR description that I can check off as I accomplish tasks, and after I'm ready to move the PR out of Draft phase, remove the TODO list from the description.

Make #218. Sorry about all the confusion. I appreciate the github pointers. I think this PR is now ready.