Improve test coverage #123

dholth · 2023-09-25T22:08:15Z

Description

Improve test coverage; include refactors found during testing.

So far, (printed in the CI run outputs),

 ---------- coverage: platform linux, python 3.10.13-final-0 ----------
Name                                 Stmts   Miss  Cover
--------------------------------------------------------
conda_index/__init__.py                  1      0   100%
conda_index/__main__.py                  2      0   100%
conda_index/api.py                      12      0   100%
conda_index/cli/__init__.py             35      0   100%
conda_index/index/__init__.py          547     17    97%
conda_index/index/common.py              8      0   100%
conda_index/index/convert_cache.py     105     30    71%
conda_index/index/logutil.py             3      0   100%
conda_index/index/rss.py                39      0   100%
conda_index/index/sqlitecache.py       253     33    87%
conda_index/utils.py                    29      4    86%
conda_index/utils_build.py             236    169    28%
conda_index/yaml.py                      9      0   100%
--------------------------------------------------------
TOTAL                                 1279    253    80%

dholth · 2023-09-26T14:25:37Z

conda_index/index/__init__.py


-        with logging_context:


this section was dedented since our logging_context, inherited from conda-build, is a no-op

…egacy cache

dholth · 2023-09-26T16:12:48Z

conda_index/index/convert_cache.py

@@ -167,29 +166,6 @@ def basename(path):
        )


-def extract_cache(path):


Converting archives of the old cache, only needed during development of new-conda-index.

kenodegard

Minor suggestions

Not a fan of the lengthy SQL statements in test_index, would it be cleaner to provide the mappings as parameters to the execute methods?

kenodegard · 2023-10-11T20:04:18Z

tests/test_index.py

+        threads=1,
+    )
+
+    instructions = Path(__file__).parents[1] / "tests" / "gen_patch_2.py"


Suggested change

instructions = Path(__file__).parents[1] / "tests" / "gen_patch_2.py"

instructions = Path(__file__).parent / "gen_patch_2.py"

Is parents[0] the same as parent, we need the second parent[1] instead

why do we need the second parent? gen_patch_2.py is right next to this file in the same directory?

I see what you mean.

kenodegard · 2023-10-11T20:06:49Z

conda_index/index/sqlitecache.py

+        for row in self.db.execute(
+            """
+            SELECT path, index_json FROM stat JOIN index_json USING (path)
+            WHERE stat.stage = ?
+            ORDER BY path
+            """,
+            (self.upstream_stage,),
+        ):
+            path, index_json = row


Suggested change

for row in self.db.execute(

"""

SELECT path, index_json FROM stat JOIN index_json USING (path)

WHERE stat.stage = ?

ORDER BY path

""",

(self.upstream_stage,),

):

path, index_json = row

for path, index_json in self.db.execute(

"""

SELECT path, index_json FROM stat JOIN index_json USING (path)

WHERE stat.stage = ?

ORDER BY path

""",

(self.upstream_stage,),

):

barabo

Just left you some little suggestion gifts, but otherwise I'm glad to see this coverage!

barabo · 2023-10-11T20:15:58Z

conda_index/index/__init__.py

@@ -727,29 +732,9 @@ def index_subdir(self, subdir, verbose=False, progress=False):

        cache = self.cache_for_subdir(subdir)

-        if verbose:
-            log.info("Building repodata for %s" % subdir_path)
+        log.debug("Building repodata for %s" % subdir_path)


Suggested change

log.debug("Building repodata for %s" % subdir_path)

log.debug("Building repodata for %s", subdir_path)

barabo · 2023-10-11T20:16:50Z

conda_index/index/__init__.py

@@ -1093,8 +1063,7 @@ def build_run_exports_data(self, subdir, verbose=False, progress=False):

        cache = self.cache_for_subdir(subdir)

-        if verbose:
-            log.info("Building run_exports for %s" % subdir_path)
+        log.debug("Building run_exports for %s" % subdir_path)


Suggested change

log.debug("Building run_exports for %s" % subdir_path)

log.debug("Building run_exports for %s", subdir_path)

barabo · 2023-10-11T20:20:41Z

conda_index/index/convert_cache.py

@@ -282,7 +258,7 @@ def convert_cache(conn, cache_generator):
                        },
                    )

-                else:
+                else:  # pragma: no cover
                    log.warn("Unhandled", match.groupdict())


Suggested change

log.warn("Unhandled", match.groupdict())

log.warn("Unhandled: %r", match.groupdict())

Without the %r - won't this ignore the second parameter?

barabo · 2023-10-11T20:25:47Z

tests/index_data/legacy_cache/.cache/index/dummy-package-1.0-0.tar.bz2.json

@@ -0,0 +1 @@
+{"build": "0", "build_number": 0, "depends": ["python 2.7.*"], "name": "dummy-package", "subdir": "osx-64", "timestamp": 1557771554052, "version": "1.0", "size": 2517, "md5": "fe3ca1adcd2b7799a5eba03cc774725d", "sha256": "c836fbdafcd615bdb4633264bf096ebc6c3c6977de1f9b2bf527ab6fc6415e01"}


Does this have to be a single line? If not, it's easier to spot changes to it later as pretty-printed.

Suggested change

{"build": "0", "build_number": 0, "depends": ["python 2.7.*"], "name": "dummy-package", "subdir": "osx-64", "timestamp": 1557771554052, "version": "1.0", "size": 2517, "md5": "fe3ca1adcd2b7799a5eba03cc774725d", "sha256": "c836fbdafcd615bdb4633264bf096ebc6c3c6977de1f9b2bf527ab6fc6415e01"}

{

"build": "0",

"build_number": 0,

"depends": [

"python 2.7.*"

],

"name": "dummy-package",

"subdir": "osx-64",

"timestamp": 1557771554052,

"version": "1.0",

"size": 2517,

"md5": "fe3ca1adcd2b7799a5eba03cc774725d",

"sha256": "c836fbdafcd615bdb4633264bf096ebc6c3c6977de1f9b2bf527ab6fc6415e01"

}

barabo · 2023-10-11T20:28:17Z

tests/index_data/legacy_cache/.cache/index/dummy-package-2.0-0.tar.bz2.json

@@ -0,0 +1 @@
+{"build": "0", "build_number": 0, "depends": ["python 2.7.*"], "name": "dummy-package", "subdir": "osx-64", "timestamp": 1557771563320, "version": "2.0", "size": 2520, "md5": "621277e18c2627643b156a0f3fad3082", "sha256": "8c2642ca7d68523bda51683703c01cdd5a232bb56f80cf947bfac458c35a5df7"}


Same here.

Suggested change

{"build": "0", "build_number": 0, "depends": ["python 2.7.*"], "name": "dummy-package", "subdir": "osx-64", "timestamp": 1557771563320, "version": "2.0", "size": 2520, "md5": "621277e18c2627643b156a0f3fad3082", "sha256": "8c2642ca7d68523bda51683703c01cdd5a232bb56f80cf947bfac458c35a5df7"}

{

"build": "0",

"build_number": 0,

"depends": [

"python 2.7.*"

],

"name": "dummy-package",

"subdir": "osx-64",

"timestamp": 1557771563320,

"version": "2.0",

"size": 2520,

"md5": "621277e18c2627643b156a0f3fad3082",

"sha256": "8c2642ca7d68523bda51683703c01cdd5a232bb56f80cf947bfac458c35a5df7"

}

barabo · 2023-10-11T20:30:38Z

tests/index_data/legacy_cache/.cache/recipe/dummy-package-1.0-0.tar.bz2.json

@@ -0,0 +1 @@
+{"package": {"name": "dummy-package", "version": "1.0"}, "build": {"number": 0, "string": "0"}, "requirements": {"run": ["python=2.7"]}, "extra": {"copy_test_source_files": true, "final": true}}


Suggested change

{"package": {"name": "dummy-package", "version": "1.0"}, "build": {"number": 0, "string": "0"}, "requirements": {"run": ["python=2.7"]}, "extra": {"copy_test_source_files": true, "final": true}}

{

"package": {

"name": "dummy-package",

"version": "1.0"

},

"build": {

"number": 0,

"string": "0"

},

"requirements": {

"run": [

"python=2.7"

]

},

"extra": {

"copy_test_source_files": true,

"final": true

}

}

barabo · 2023-10-11T20:35:12Z

tests/index_data/legacy_cache/.cache/recipe/dummy-package-2.0-0.tar.bz2.json

@@ -0,0 +1 @@
+{"package": {"name": "dummy-package", "version": "2.0"}, "build": {"number": 0, "string": "0"}, "requirements": {"run": ["python=2.7"]}, "extra": {"copy_test_source_files": true, "final": true}}


Suggested change

{"package": {"name": "dummy-package", "version": "2.0"}, "build": {"number": 0, "string": "0"}, "requirements": {"run": ["python=2.7"]}, "extra": {"copy_test_source_files": true, "final": true}}

{

"package": {

"name": "dummy-package",

"version": "2.0"

},

"build": {

"number": 0,

"string": "0"

},

"requirements": {

"run": [

"python=2.7"

]

},

"extra": {

"copy_test_source_files": true,

"final": true

}

}

barabo · 2023-10-11T20:35:51Z

tests/index_data/legacy_cache/.cache/stat.json

@@ -0,0 +1 @@
+{"dummy-package-1.0-0.tar.bz2": {"mtime": 1672933250, "size": 2517}, "dummy-package-2.0-0.tar.bz2": {"mtime": 1672933250, "size": 2520}}


Suggested change

{"dummy-package-1.0-0.tar.bz2": {"mtime": 1672933250, "size": 2517}, "dummy-package-2.0-0.tar.bz2": {"mtime": 1672933250, "size": 2520}}

{

"dummy-package-1.0-0.tar.bz2": {

"mtime": 1672933250,

"size": 2517

},

"dummy-package-2.0-0.tar.bz2": {

"mtime": 1672933250,

"size": 2520

}

}

barabo · 2023-10-11T20:40:47Z

tests/test_sqlitecache.py

+    convert_cache(conn, extract_cache_filesystem(legacy_cache))
+    assert conn.execute("SELECT COUNT(*) FROM index_json").fetchone()[0] == 2


I would have expected the context manager to have closed the conn connection above. Would this also work?

Suggested change

convert_cache(conn, extract_cache_filesystem(legacy_cache))

assert conn.execute("SELECT COUNT(*) FROM index_json").fetchone()[0] == 2

convert_cache(conn, extract_cache_filesystem(legacy_cache))

assert conn.execute("SELECT COUNT(*) FROM index_json").fetchone()[0] == 2

barabo · 2023-10-11T20:50:07Z

tests/test_index.py

+        conn.execute(
+            f"""INSERT INTO index_json VALUES('{features_pkg}','{{"build":"h39de5ba_0","build_number":0,"depends":[],"name":"{features_pkg_name}","noarch":"generic","subdir":"noarch","timestamp":1561127261940,"version":"1.0","md5":"ba68433ef44982170d4e2f2f9bf89338","sha256":"33877cbe447e8c7a026fbcb7e299b37208ad4bc70cf8328fb4cf552af01ada76","size":2683,"track_features":["jim"],"features":["jim"]}}');"""
+        )


Suggested change

conn.execute(

f"""INSERT INTO index_json VALUES('{features_pkg}','{{"build":"h39de5ba_0","build_number":0,"depends":[],"name":"{features_pkg_name}","noarch":"generic","subdir":"noarch","timestamp":1561127261940,"version":"1.0","md5":"ba68433ef44982170d4e2f2f9bf89338","sha256":"33877cbe447e8c7a026fbcb7e299b37208ad4bc70cf8328fb4cf552af01ada76","size":2683,"track_features":["jim"],"features":["jim"]}}');"""

)

features_pkg_json = json.dumps(

{

"build": "h39de5ba_0",

"build_number": 0,

"depends": [],

"name": features_pkg_name,

"noarch": "generic",

"subdir": "noarch",

"timestamp": 1561127261940,

"version": "1.0",

"md5": "ba68433ef44982170d4e2f2f9bf89338",

"sha256": "33877cbe447e8c7a026fbcb7e299b37208ad4bc70cf8328fb4cf552af01ada76",

"size": 2683,

"track_features": [

"jim"

],

"features": [

"jim"

]

}

)

conn.execute(

f"""INSERT INTO index_json VALUES('{features_pkg}',{features_pkg_json});"""

)

dholth added 4 commits September 25, 2023 13:03

remove 'if verbose' check

3fcf2db

remove unused function

f8950d5

improve test coverage; add some type annotations

8ad77ac

move indexed packages query into sqlitecache

64b5ef2

dholth requested a review from jezdez September 25, 2023 22:08

dholth linked an issue Sep 25, 2023 that may be closed by this pull request

Coverage tests are less than 100% #114

Closed

2 tasks

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Sep 25, 2023

dholth added 2 commits September 25, 2023 18:22

test incompatible patch

20b69c9

additional tests

6845e98

dholth commented Sep 26, 2023

View reviewed changes

dholth requested a review from barabo September 26, 2023 14:47

improve coverage; remove code related to converting tar archives of l…

76c37cc

…egacy cache

dholth commented Sep 26, 2023

View reviewed changes

dholth enabled auto-merge (squash) October 11, 2023 20:14

kenodegard reviewed Oct 11, 2023

View reviewed changes

barabo approved these changes Oct 11, 2023

View reviewed changes

dholth merged commit 3fcd25a into main Oct 11, 2023
7 checks passed

dholth deleted the improve-test-coverage branch October 11, 2023 20:52

github-actions bot added the locked [bot] locked due to inactivity label Oct 11, 2024

github-actions bot locked as resolved and limited conversation to collaborators Oct 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve test coverage #123

Improve test coverage #123

dholth commented Sep 25, 2023 •

edited

Loading

dholth Sep 26, 2023

dholth Sep 26, 2023

kenodegard left a comment

kenodegard Oct 11, 2023

dholth Oct 11, 2023 •

edited

Loading

kenodegard Oct 11, 2023

dholth Oct 12, 2023

kenodegard Oct 11, 2023

barabo left a comment

barabo Oct 11, 2023

barabo Oct 11, 2023

barabo Oct 11, 2023

barabo Oct 11, 2023

barabo Oct 11, 2023

barabo Oct 11, 2023

barabo Oct 11, 2023

barabo Oct 11, 2023

barabo Oct 11, 2023

barabo Oct 11, 2023

		@@ -167,29 +166,6 @@ def basename(path):
		)


		def extract_cache(path):

	instructions = Path(__file__).parents[1] / "tests" / "gen_patch_2.py"
	instructions = Path(__file__).parent / "gen_patch_2.py"

	log.debug("Building repodata for %s" % subdir_path)
	log.debug("Building repodata for %s", subdir_path)

	log.debug("Building run_exports for %s" % subdir_path)
	log.debug("Building run_exports for %s", subdir_path)

	log.warn("Unhandled", match.groupdict())
	log.warn("Unhandled: %r", match.groupdict())

		@@ -0,0 +1 @@
		{"build": "0", "build_number": 0, "depends": ["python 2.7.*"], "name": "dummy-package", "subdir": "osx-64", "timestamp": 1557771554052, "version": "1.0", "size": 2517, "md5": "fe3ca1adcd2b7799a5eba03cc774725d", "sha256": "c836fbdafcd615bdb4633264bf096ebc6c3c6977de1f9b2bf527ab6fc6415e01"}

		@@ -0,0 +1 @@
		{"package": {"name": "dummy-package", "version": "1.0"}, "build": {"number": 0, "string": "0"}, "requirements": {"run": ["python=2.7"]}, "extra": {"copy_test_source_files": true, "final": true}}

		@@ -0,0 +1 @@
		{"dummy-package-1.0-0.tar.bz2": {"mtime": 1672933250, "size": 2517}, "dummy-package-2.0-0.tar.bz2": {"mtime": 1672933250, "size": 2520}}

		convert_cache(conn, extract_cache_filesystem(legacy_cache))
		assert conn.execute("SELECT COUNT(*) FROM index_json").fetchone()[0] == 2

-        conn.execute(
-            f"""INSERT INTO index_json VALUES('{features_pkg}','{{"build":"h39de5ba_0","build_number":0,"depends":[],"name":"{features_pkg_name}","noarch":"generic","subdir":"noarch","timestamp":1561127261940,"version":"1.0","md5":"ba68433ef44982170d4e2f2f9bf89338","sha256":"33877cbe447e8c7a026fbcb7e299b37208ad4bc70cf8328fb4cf552af01ada76","size":2683,"track_features":["jim"],"features":["jim"]}}');"""
-        )
+        features_pkg_json = json.dumps(
+            {
+              "build": "h39de5ba_0",
+              "build_number": 0,
+              "depends": [],
+              "name": features_pkg_name,
+              "noarch": "generic",
+              "subdir": "noarch",
+              "timestamp": 1561127261940,
+              "version": "1.0",
+              "md5": "ba68433ef44982170d4e2f2f9bf89338",
+              "sha256": "33877cbe447e8c7a026fbcb7e299b37208ad4bc70cf8328fb4cf552af01ada76",
+              "size": 2683,
+              "track_features": [
+                "jim"
+              ],
+              "features": [
+                "jim"
+              ]
+            }
+        )
+        conn.execute(
+            f"""INSERT INTO index_json VALUES('{features_pkg}',{features_pkg_json});"""
+        )

Improve test coverage #123

Improve test coverage #123

Conversation

dholth commented Sep 25, 2023 • edited Loading

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenodegard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dholth Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barabo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dholth commented Sep 25, 2023 •

edited

Loading

dholth Oct 11, 2023 •

edited

Loading