pitrou commented on code in PR #50669: URL: https://github.com/apache/arrow/pull/50669#discussion_r3664177813
########## dev/archery/archery/crossbow/tests/test_core.py: ########## @@ -17,16 +17,103 @@ from archery.utils.source import ArrowSources from archery.crossbow import Config, Queue -from archery.crossbow.core import CrossbowError, Repo, TaskAssets, TaskStatus +from archery.crossbow.core import ( + CrossbowError, + Repo, + TaskAssets, + TaskStatus, + get_version, +) import pathlib +import subprocess from datetime import date from unittest import mock import pytest from github import GithubException [email protected]( + ("description", "expected"), + [ + ("apache-arrow-4.0.0-0-gabcdef\n", "5.0.0.dev0"), + ("apache-arrow-4.0.0-12-gabcdef-dirty\n", "5.0.0.dev12"), + ("apache-arrow-5.0.0.dev-7-gabcdef\n", "5.0.0.dev7"), + ("apache-arrow-5.0.0-rc1-2-gabcdef\n", "6.0.0.dev2"), + ], +) +def test_get_version(description, expected): + with mock.patch("archery.crossbow.core.subprocess.run") as mocked_run: + mocked_run.return_value.stdout = description + + assert get_version("/arrow") == expected + + mocked_run.assert_called_once_with( + [ + "git", + "describe", + "--dirty", + "--tags", + "--long", + "--match", + "apache-arrow-[0-9]*.*", + ], + cwd="/arrow", + check=True, + stdout=mock.ANY, + text=True, + ) + + +def test_get_version_rejects_unexpected_describe_output(): + with mock.patch("archery.crossbow.core.subprocess.run") as mocked_run: + mocked_run.return_value.stdout = "not-an-arrow-version\n" + + with pytest.raises(CrossbowError, match="git describe output"): + get_version("/arrow") + + +def test_get_version_from_git_repository(tmp_path): + def run_git(*args): + subprocess.run( + ["git", *args], + cwd=tmp_path, + check=True, + capture_output=True, + text=True, + ) + + run_git("init") + version_file = tmp_path / "version.txt" + version_file.write_text("released\n") + run_git("add", "version.txt") Review Comment: What is the version file for? It shouldn't be used for version parsing AFAICT. ########## dev/archery/archery/crossbow/tests/test_core.py: ########## @@ -17,16 +17,103 @@ from archery.utils.source import ArrowSources from archery.crossbow import Config, Queue -from archery.crossbow.core import CrossbowError, Repo, TaskAssets, TaskStatus +from archery.crossbow.core import ( + CrossbowError, + Repo, + TaskAssets, + TaskStatus, + get_version, +) import pathlib +import subprocess from datetime import date from unittest import mock import pytest from github import GithubException [email protected]( + ("description", "expected"), + [ + ("apache-arrow-4.0.0-0-gabcdef\n", "5.0.0.dev0"), + ("apache-arrow-4.0.0-12-gabcdef-dirty\n", "5.0.0.dev12"), + ("apache-arrow-5.0.0.dev-7-gabcdef\n", "5.0.0.dev7"), + ("apache-arrow-5.0.0-rc1-2-gabcdef\n", "6.0.0.dev2"), + ], +) +def test_get_version(description, expected): + with mock.patch("archery.crossbow.core.subprocess.run") as mocked_run: + mocked_run.return_value.stdout = description + + assert get_version("/arrow") == expected + + mocked_run.assert_called_once_with( + [ + "git", + "describe", + "--dirty", + "--tags", + "--long", + "--match", + "apache-arrow-[0-9]*.*", + ], + cwd="/arrow", + check=True, + stdout=mock.ANY, + text=True, + ) + + +def test_get_version_rejects_unexpected_describe_output(): + with mock.patch("archery.crossbow.core.subprocess.run") as mocked_run: + mocked_run.return_value.stdout = "not-an-arrow-version\n" + + with pytest.raises(CrossbowError, match="git describe output"): + get_version("/arrow") + + +def test_get_version_from_git_repository(tmp_path): + def run_git(*args): Review Comment: This test will fail if a `git` command is not available (which is unlikely on dev setups, for sure). Can we skip in that case? ########## dev/archery/archery/crossbow/core.py: ########## @@ -17,6 +17,7 @@ import os import re +import subprocess Review Comment: Nit: please let's keep these imports (roughly) alphabetically-ordered :-) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
