addurls: Provide better handling of invalid and empty input

[kyleam]

Fixes gh-3577 and another gh-3577-inspired issue.

[yarikoptic]

Just a minor recommendation

[yarikoptic]

Just a minor recommendation

[yarikoptic]

Could have just use with_tree to get temporary directory with a file with content. IMHO would have been more descriptive and shorter

[kyleam]

Yeah, I like {with,create}_tree too. I'm not sure why I didn't use it here. Updated.

range-diff

1:  d853daad3 = 1:  d853daad3 DOC: addurls: Update stale return value
2:  cc54a8f1a ! 2:  8c8f2a9a2 BF: addurls: Provide better error for invalid input stream
    @@ -48,19 +48,23 @@
      
      from datalad.api import addurls, Dataset, subdatasets
      import datalad.plugin.addurls as au
    +@@
    + from datalad.tests.utils import assert_dict_equal
    + from datalad.tests.utils import eq_, ok_exists
    + from datalad.tests.utils import create_tree, with_tempfile, HTTPPath
    ++from datalad.tests.utils import with_tree
    + from datalad.utils import get_tempfile_kwargs, rmtemp
    + 
    + 
     @@
                   op.join("bar", "adir", "bar-again", "other-ds")})
              ok_exists(os.path.join(
                  ds.path, "foo", "adir", "foo-again", "other-ds", "bdir", "a"))
     +
    -+    @with_tempfile(mkdir=True)
    ++    @with_tree({"in": ""})
     +    def test_addurls_invalid_input(self, path):
    -+        ds = Dataset(path).create()
    -+
    ++        ds = Dataset(path).create(force=True)
     +        in_file = op.join(path, "in")
    -+        with open(in_file, "w") as fh:
    -+            fh.write("")
    -+
     +        for in_type in ["csv", "json"]:
     +            with assert_raises(IncompleteResultsError) as exc:
     +                ds.addurls(in_file, "{url}", "{name}", input_type=in_type)
3:  ae358d7b5 ! 3:  de086917b BF: addurls: Don't assume that the stream has rows
    @@ -32,19 +32,13 @@
                      ds.addurls(in_file, "{url}", "{name}", input_type=in_type)
                  assert_in("Failed to read", text_type(exc.exception))
     +
    -+    @with_tempfile(mkdir=True)
    ++    @with_tree({"in.csv": "url,name,subdir",
    ++                "in.json": "[]"})
     +    def test_addurls_no_rows(self, path):
    -+        ds = Dataset(path).create()
    -+
    -+        in_csv = op.join(path, "in.csv")
    -+        with open(in_csv, "w") as fh:
    -+            fh.write("url,name,subdir")
    -+
    -+        in_json = op.join(path, "in.json")
    -+        with open(in_json, "w") as fh:
    -+            fh.write("[]")
    -+
    -+        for fname in [in_csv, in_json]:
    ++        ds = Dataset(path).create(force=True)
    ++        for fname in ["in.csv", "in.json"]:
    ++            # TODO: This op.join() can be dropped once gh-3580 is fixed.
    ++            fname = op.join(path, fname)
     +            with swallow_logs(new_level=logging.WARNING) as cml:
     +                ds.addurls(fname, "{url}", "{name}")
     +                cml.assert_logged("No rows", regex=False)
4:  355dc8a72 ! 4:  281512ca1 RF: addurls: Return early if there are no rows to process
    @@ -27,8 +27,8 @@
      --- a/datalad/plugin/tests/test_addurls.py
      +++ b/datalad/plugin/tests/test_addurls.py
     @@
    - 
    -         for fname in [in_csv, in_json]:
    +             # TODO: This op.join() can be dropped once gh-3580 is fixed.
    +             fname = op.join(path, fname)
                  with swallow_logs(new_level=logging.WARNING) as cml:
     -                ds.addurls(fname, "{url}", "{name}")
     +                assert_in_results(

[yarikoptic]

Same here