http: raise error on unsuccessful http statuses on exists

[damakuno]

• ❗ I have followed the Contributing to DVC checklist.

• 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.

This PR is to fix #4288 . It is still a work in progress as I haven't written any tests for the changes I have implemented, and I need some guidance on where/how to write tests for the project. Do I add to the unit test script in dvc/tests/unit/remote/test_http.py?

I'm not sure if this requires documentation, but I'll create a PR for it if needed.

[damakuno]

I'm a bit stumped on writing the tests. If my understanding is correct I need to test 3 scenarios:

1. When the response returns 404
2. When the response returns 200 OK
3. Other types of responses that will raise a HTTPError

I'm just not sure how to mock responses from the requests module?
It's my first time writing tests for this project, any pointers are very much appreciated.
I'm thinking about something like this, with comments on things I have yet to implement.
in dvc/tests/unit/remote/test_http.py:

...
def test_exists(dvc):
    from dvc.path_info import URLInfo
    config = {
        "url": "http://127.0.0.1",
        "path_info": "file.html"
    }
    path_info = URLInfo(config["url"])
    tree = HTTPTree(dvc, config)

    # mock a successful response that returns 200 OK
    assert tree.exists(path_info) is True
    # mock a response that returns 404
    assert tree.exists(path_info) is False
    # mock a HTTP Error response that is not 404 to 
    with pytest.raises(HTTPError):
        tree.exists(path_info)

P.S. I'll keep digging and try to figure something out, so I'm still working on it!

[skshetry]

@damakuno, sadly we don't have a good way to test errors in http remote, as we are using this hack for tests:
https://github.com/iterative/dvc/blob/59d5c73c5e5fd6e0c5bba7f6fc7ba5677380ed40/tests/utils/httpd.py#L11

The corresponding pytest http fixture is here.

The short-term solution can be mocking, but it's a code smell. But, it's the only one that I see for now:

def test_exists(dvc, http, mocker):
    tree = HTTPTree(dvc, http.config)

    resp = requests.Response()
    resp.status_code = requests.codes.FORBIDDEN
    resp.raw = io.StringIO("boo!!!")

    mocker.patch.object(tree, "request", return_value=resp)
    with pytest.raises(HTTPError):
        tree.exists(http / "file.txt")

[damakuno]

How do I resolve the failing code coverage check? I'm not quite sure what's the issue

[skshetry]

@damakuno, nothing to do with the coverage, as you have already added the tests.

[skshetry]

Thanks, @damakuno for contributing. The changes look good, and can be merged. I added a comment to clarify on the test. 🎉