Repository: astropy/pyvo

Issue:
MAINT: improve test coverage of regtap
Currently, it's only 55% with missing coverage of numerous methods leading to wonder whether hash-based splitting of standardid works as intended for services with IDs including a hash, e.g. the distinction of SIA and SIA2.


https://github.com/astropy/pyvo/blob/main/pyvo/registry/regtap.py#L311
https://github.com/astropy/pyvo/blob/main/pyvo/registry/regtap.py#L338

Discussion:
codecov[bot]: ## [Codecov](https://codecov.io/gh/astropy/pyvo/pull/427?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astropy) Report
> Merging [#427](https://codecov.io/gh/astropy/pyvo/pull/427?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astropy) (cb425f5) into [main](https://codecov.io/gh/astropy/pyvo/commit/a89748389b66f5f69afc14cb76652060538e42d6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astropy) (a897483) will **increase** coverage by `0.12%`.
> The diff coverage is `85.71%`.

```diff
@@            Coverage Diff             @@
##             main     #427      +/-   ##
==========================================
+ Coverage   79.94%   80.07%   +0.12%     
==========================================
  Files          52       52              
  Lines        6018     6022       +4     
==========================================
+ Hits         4811     4822      +11     
+ Misses       1207     1200       -7     
```


| [Impacted Files](https://codecov.io/gh/astropy/pyvo/pull/427?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astropy) | Coverage Δ | |
|---|---|---|
| [pyvo/registry/regtap.py](https://codecov.io/gh/astropy/pyvo/pull/427?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astropy#diff-cHl2by9yZWdpc3RyeS9yZWd0YXAucHk=) | `82.75% <85.71%> (+2.99%)` | :arrow_up: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astropy)


Patch:
diff --git a/pyvo/registry/regtap.py b/pyvo/registry/regtap.py
index 4fb5f2c..89f1355 100644
--- a/pyvo/registry/regtap.py
+++ b/pyvo/registry/regtap.py
@@ -25,6 +25,8 @@ from astropy import table
 from astropy.utils.decorators import deprecated
 from astropy.utils.exceptions import AstropyDeprecationWarning
 
+import numpy
+
 from . import rtcons
 from ..dal import scs, sia, sia2, ssa, sla, tap, query as dalq
 from ..io.vosi import vodataservice
@@ -539,7 +541,11 @@ class RegistryResource(dalq.Record):
         by which a positional query against this resource should be "blurred"
         in order to get an appropriate match.
         """
-        return float(self.get("region_of_regard", 0))
+        # we get NULLs as NaNs here
+        val = self["region_of_regard"]
+        if numpy.isnan(val):
+            return None
+        return val
 
     @property
     def waveband(self):
@@ -734,15 +740,16 @@ class RegistryResource(dalq.Record):
 
         Raises
         ------
-        RuntimeError
+        DALServiceError
            if the resource does not describe a searchable service.
         """
-        if not self.service:
+        try:
+            return self.service.search(*args, **keys)
+        except ValueError:
+            # I blindly assume the ValueError comes out of get_interface.
+            # But then that's likely enough.
             raise dalq.DALServiceError(
-                "resource, {}, is not a searchable service".format(
-                    self.short_name))
-
-        return self.service.search(*args, **keys)
+                f"Resource {self.ivoid} is not a searchable service")
 
     def describe(self, verbose=False, width=78, file=None):
         """


Test patch:
diff --git a/pyvo/registry/tests/test_regtap.py b/pyvo/registry/tests/test_regtap.py
index a534c70..622604c 100644
--- a/pyvo/registry/tests/test_regtap.py
+++ b/pyvo/registry/tests/test_regtap.py
@@ -383,6 +383,7 @@ def test_record_fields(rt_pulsar_distance):
     assert rec.content_types == ['catalog']
     assert rec.source_format == "bibcode"
     assert rec.source_value == "1993ApJS...88..529T"
+    assert rec.region_of_regard is None
     assert rec.waveband == ['radio']
     # access URL, standard_id and friends exercised in TestInterfaceSelection
 
@@ -453,6 +454,17 @@ class TestInterfaceSelection:
         assert (svc.access_url
                 == "http://dc.zah.uni-heidelberg.de/flashheros/q/web/form")
 
+        import webbrowser
+        orig_open = webbrowser.open
+        try:
+            open_args = []
+            webbrowser.open = lambda *args: open_args.append(args)
+            svc.search()
+            assert open_args == [
+                ("http://dc.zah.uni-heidelberg.de/flashheros/q/web/form", 2)]
+        finally:
+            webbrowser.open = orig_open
+
     def test_get_aux_interface(self, flash_service):
         svc = flash_service.get_service("tap#aux")
         assert (svc._baseurl
@@ -540,6 +552,22 @@ class TestInterfaceSelection:
         assert rec.get_interface("sia2").access_url == 'http://sia2.example.com'
         assert rec.get_interface("sia").access_url == 'http://sia.example.com'
 
+    def test_non_standard_interface(self):
+        intf = regtap.Interface("http://url", "", "", "")
+        assert intf.supports("ivo://ivoa.net/std/sia") is False
+
+    def test_supports_none(self):
+        intf = regtap.Interface("http://url", "", "", "")
+        assert intf.supports(None) is False
+
+    def test_non_searchable_service(self):
+        rec = _makeRegistryRecord()
+        with pytest.raises(dalq.DALServiceError) as excinfo:
+            rec.search()
+
+        assert str(excinfo.value) == (
+            "Resource ivo://pyvo/test_regtap.py is not a searchable service")
+
 
 class _FakeResult:
     """A fake class just sufficient for giving dal.query.Record enough
@@ -623,6 +651,9 @@ class TestInterfaceRejection:
             intf_roles=["", "std"])
 
         assert (rsc.service._baseurl == "http://b")
+        # this makes sure caching the service obtained doesn't break
+        # things
+        assert (rsc.service._baseurl == "http://b")
 
     def test_capless(self):
         rsc = _makeRegistryRecord()