launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30179
[Merge] ~cjwatson/launchpad:remove-lp-services-scripts-tests-run-script into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:remove-lp-services-scripts-tests-run-script into launchpad:master.
Commit message:
Remove lp.services.scripts.tests.run_script
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/445957
It turned out that the only way in which `lp.testing.script.run_script`'s `env` argument was ever used was to supplement the environment of the calling process, so I changed it to use the simpler `extra_env` calling convention from `lp.services.scripts.tests.run_script`. Aside from that, the main difference was that `lp.services.scripts.tests.run_script` tested the exit code itself, and it's probably clearer to have that be explicit in tests anyway.
(Note that some tests already had their own separate assertion of the exit code, although it isn't always visible in the patch context.)
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remove-lp-services-scripts-tests-run-script into launchpad:master.
diff --git a/lib/launchpad_loggerhead/testing.py b/lib/launchpad_loggerhead/testing.py
index 0847bf1..7d24ecc 100644
--- a/lib/launchpad_loggerhead/testing.py
+++ b/lib/launchpad_loggerhead/testing.py
@@ -18,8 +18,8 @@ from lp.services.osutils import (
remove_if_exists,
)
from lp.services.pidfile import pidfile_path
-from lp.services.scripts.tests import run_script
from lp.testing.layers import BaseLayer, LayerProcessController
+from lp.testing.script import run_script
class LoggerheadFixtureException(Exception):
@@ -43,14 +43,19 @@ class LoggerheadFixture(Fixture):
self.logfile = os.path.join(config.codebrowse.log_folder, "debug.log")
remove_if_exists(self.logfile)
self.addCleanup(kill_by_pidfile, pidfile)
- run_script(
- os.path.join("scripts", "start-loggerhead.py"),
- ["--daemon"],
+ exit_code, out, err = run_script(
+ os.path.join(config.root, "scripts", "start-loggerhead.py"),
+ args=["--daemon"],
# The testrunner-appserver config provides the correct
# openid_provider_root URL.
extra_env={"LPCONFIG": BaseLayer.appserver_config_name},
universal_newlines=False,
)
+ if exit_code != 0:
+ raise AssertionError(
+ "Starting loggerhead failed with exit code %d:\n%s\n%s"
+ % (exit_code, out, err)
+ )
self._waitForStartup()
def _hasStarted(self):
diff --git a/lib/lp/answers/tests/test_questionjob.py b/lib/lp/answers/tests/test_questionjob.py
index 3c3a7f7..5d7451b 100644
--- a/lib/lp/answers/tests/test_questionjob.py
+++ b/lib/lp/answers/tests/test_questionjob.py
@@ -3,8 +3,6 @@
"""Tests for QuestionJobs classes."""
-import os
-
import transaction
from testtools.content import text_content
from zope.component import getUtility
@@ -355,12 +353,10 @@ class QuestionEmailJobTestCase(TestCaseWithFactory):
question.target.addAnswerContact(user, user)
transaction.commit()
- env = os.environ.copy()
- env["LP_DEBUG_SQL"] = "1"
exit_code, out, err = run_script(
"cronscripts/process-job-source.py",
args=["-vv", IQuestionEmailJobSource.getName()],
- env=env,
+ extra_env={"LP_DEBUG_SQL": "1"},
)
self.addDetail("stdout", text_content(out))
self.addDetail("stderr", text_content(err))
diff --git a/lib/lp/archivepublisher/tests/test_generate_contents_files.py b/lib/lp/archivepublisher/tests/test_generate_contents_files.py
index 8dfaee5..838f59f 100644
--- a/lib/lp/archivepublisher/tests/test_generate_contents_files.py
+++ b/lib/lp/archivepublisher/tests/test_generate_contents_files.py
@@ -20,11 +20,11 @@ from lp.registry.interfaces.series import SeriesStatus
from lp.services.log.logger import DevNullLogger
from lp.services.osutils import write_file
from lp.services.scripts.base import LaunchpadScriptFailure
-from lp.services.scripts.tests import run_script
from lp.services.utils import file_exists
from lp.testing import TestCaseWithFactory
from lp.testing.faketransaction import FakeTransaction
from lp.testing.layers import LaunchpadZopelessLayer, ZopelessDatabaseLayer
+from lp.testing.script import run_script
def fake_overrides(script, distroseries):
@@ -337,6 +337,7 @@ class TestGenerateContentsFiles(TestCaseWithFactory):
# The script will run stand-alone.
self.layer.force_dirty_database()
retval, out, err = run_script(
- "cronscripts/generate-contents-files.py", ["-d", "ubuntu", "-q"]
+ "cronscripts/generate-contents-files.py",
+ args=["-d", "ubuntu", "-q"],
)
self.assertEqual(0, retval)
diff --git a/lib/lp/bugs/tests/test_apportjob.py b/lib/lp/bugs/tests/test_apportjob.py
index 96cd204..370baf6 100644
--- a/lib/lp/bugs/tests/test_apportjob.py
+++ b/lib/lp/bugs/tests/test_apportjob.py
@@ -23,7 +23,6 @@ from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.tests import block_on_job
from lp.services.librarian.interfaces import ILibraryFileAliasSet
-from lp.services.scripts.tests import run_script
from lp.services.temporaryblobstorage.interfaces import (
ITemporaryStorageManager,
)
@@ -34,6 +33,7 @@ from lp.testing.layers import (
LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
)
+from lp.testing.script import run_script
from lp.testing.views import create_initialized_view
@@ -304,9 +304,9 @@ class ProcessApportBlobJobTestCase(TestCaseWithFactory):
retcode, stdout, stderr = run_script(
"cronscripts/process-job-source.py",
- ["IProcessApportBlobJobSource"],
- expect_returncode=0,
+ args=["IProcessApportBlobJobSource"],
)
+ self.assertEqual(0, retcode)
self.assertEqual("", stdout)
self.assertIn("INFO Ran 1 ProcessApportBlobJob jobs.\n", stderr)
diff --git a/lib/lp/code/scripts/tests/test_merge_proposal_jobs.py b/lib/lp/code/scripts/tests/test_merge_proposal_jobs.py
index f571899..7082934 100644
--- a/lib/lp/code/scripts/tests/test_merge_proposal_jobs.py
+++ b/lib/lp/code/scripts/tests/test_merge_proposal_jobs.py
@@ -5,6 +5,8 @@
"""Test the sendbranchmail script"""
+import os.path
+
import transaction
from testtools.matchers import MatchesRegex
@@ -12,9 +14,10 @@ from lp.code.model.tests.test_branchmergeproposaljobs import (
make_runnable_incremental_diff_job,
)
from lp.code.model.tests.test_diff import DiffTestCase
+from lp.services.config import config
from lp.services.job.interfaces.job import JobStatus
-from lp.services.scripts.tests import run_script
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
class TestMergeProposalJobScript(DiffTestCase):
@@ -26,8 +29,8 @@ class TestMergeProposalJobScript(DiffTestCase):
job = make_runnable_incremental_diff_job(self)
transaction.commit()
retcode, stdout, stderr = run_script(
- "cronscripts/process-job-source.py",
- ["--log-twisted", "IBranchMergeProposalJobSource"],
+ os.path.join(config.root, "cronscripts", "process-job-source.py"),
+ args=["--log-twisted", "IBranchMergeProposalJobSource"],
)
self.assertEqual(0, retcode)
self.assertEqual("", stdout)
diff --git a/lib/lp/code/scripts/tests/test_reclaim_branch_space.py b/lib/lp/code/scripts/tests/test_reclaim_branch_space.py
index 9a85f77..d30ac09 100644
--- a/lib/lp/code/scripts/tests/test_reclaim_branch_space.py
+++ b/lib/lp/code/scripts/tests/test_reclaim_branch_space.py
@@ -14,9 +14,9 @@ from lp.code.model.branchjob import BranchJob, BranchJobType
from lp.codehosting.vfs import branch_id_to_path
from lp.services.config import config
from lp.services.database.interfaces import IStore
-from lp.services.scripts.tests import run_script
from lp.testing import TestCaseWithFactory
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
class TestReclaimBranchSpaceScript(TestCaseWithFactory):
diff --git a/lib/lp/code/scripts/tests/test_request_daily_builds.py b/lib/lp/code/scripts/tests/test_request_daily_builds.py
index bf003bc..24f4266 100644
--- a/lib/lp/code/scripts/tests/test_request_daily_builds.py
+++ b/lib/lp/code/scripts/tests/test_request_daily_builds.py
@@ -19,11 +19,11 @@ from lp.code.interfaces.codehosting import BRANCH_ID_ALIAS_PREFIX
from lp.services.config import config
from lp.services.config.fixture import ConfigFixture, ConfigUseFixture
from lp.services.features.testing import FeatureFixture
-from lp.services.scripts.tests import run_script
from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS, ISnap
from lp.soyuz.enums import ArchivePurpose
from lp.testing import TestCaseWithFactory
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
class SilentWSGIRequestHandler(WSGIRequestHandler):
@@ -329,8 +329,9 @@ class TestRequestDailyBuilds(TestCaseWithFactory):
pack_ref.repository, "charmcraft.yaml", b"name: pack-charm"
)
retcode, stdout, stderr = run_script(
- "cronscripts/request_daily_builds.py", []
+ "cronscripts/request_daily_builds.py"
)
+ self.assertEqual(0, retcode)
self.assertIn("Requested 4 daily recipe builds.", stderr)
self.assertIn(
"Requested 4 sets of automatic snap package builds.", stderr
@@ -353,8 +354,9 @@ class TestRequestDailyBuilds(TestCaseWithFactory):
)
transaction.commit()
retcode, stdout, stderr = run_script(
- "cronscripts/request_daily_builds.py", []
+ "cronscripts/request_daily_builds.py"
)
+ self.assertEqual(0, retcode)
self.assertEqual(0, recipe.pending_builds.count())
self.assertIn("Requested 0 daily recipe builds.", stderr)
self.assertIn(
diff --git a/lib/lp/code/scripts/tests/test_scan_branches.py b/lib/lp/code/scripts/tests/test_scan_branches.py
index 8a1e3eb..2363bb2 100644
--- a/lib/lp/code/scripts/tests/test_scan_branches.py
+++ b/lib/lp/code/scripts/tests/test_scan_branches.py
@@ -5,6 +5,7 @@
"""Test the scan_branches script."""
+import os.path
import transaction
from storm.locals import Store
@@ -15,11 +16,12 @@ from lp.code.enums import (
CodeReviewNotificationLevel,
)
from lp.code.model.branchjob import BranchJob, BranchJobType, BranchScanJob
+from lp.services.config import config
from lp.services.job.model.job import Job, JobStatus
from lp.services.osutils import override_environ
-from lp.services.scripts.tests import run_script
from lp.testing import TestCaseWithFactory
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
class TestScanBranches(TestCaseWithFactory):
@@ -42,11 +44,11 @@ class TestScanBranches(TestCaseWithFactory):
def run_script_and_assert_success(self):
"""Run the scan_branches script and assert it ran successfully."""
retcode, stdout, stderr = run_script(
- "cronscripts/process-job-source.py",
- ["IBranchScanJobSource"],
- expect_returncode=0,
+ os.path.join(config.root, "cronscripts", "process-job-source.py"),
+ args=["IBranchScanJobSource"],
)
self.oops_capture.sync()
+ self.assertEqual(0, retcode)
self.assertEqual("", stdout)
self.assertIn("INFO Ran 1 BranchScanJob jobs.\n", stderr)
diff --git a/lib/lp/code/scripts/tests/test_sendbranchmail.py b/lib/lp/code/scripts/tests/test_sendbranchmail.py
index 629f6a2..3b03be9 100644
--- a/lib/lp/code/scripts/tests/test_sendbranchmail.py
+++ b/lib/lp/code/scripts/tests/test_sendbranchmail.py
@@ -3,6 +3,8 @@
"""Test the sendbranchmail script"""
+import os.path
+
import transaction
from lp.code.enums import (
@@ -11,10 +13,11 @@ from lp.code.enums import (
CodeReviewNotificationLevel,
)
from lp.code.model.branchjob import RevisionMailJob, RevisionsAddedJob
+from lp.services.config import config
from lp.services.osutils import override_environ
-from lp.services.scripts.tests import run_script
from lp.testing import TestCaseWithFactory
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
class TestSendbranchmail(TestCaseWithFactory):
@@ -48,7 +51,8 @@ class TestSendbranchmail(TestCaseWithFactory):
)
transaction.commit()
retcode, stdout, stderr = run_script(
- "cronscripts/process-job-source.py", ["IRevisionMailJobSource"]
+ os.path.join(config.root, "cronscripts", "process-job-source.py"),
+ args=["IRevisionMailJobSource"],
)
self.assertTextMatchesExpressionIgnoreWhitespace(
"INFO "
@@ -77,7 +81,8 @@ class TestSendbranchmail(TestCaseWithFactory):
)
transaction.commit()
retcode, stdout, stderr = run_script(
- "cronscripts/process-job-source.py", ["IRevisionsAddedJobSource"]
+ os.path.join(config.root, "cronscripts", "process-job-source.py"),
+ args=["IRevisionsAddedJobSource"],
)
self.assertTextMatchesExpressionIgnoreWhitespace(
"INFO "
diff --git a/lib/lp/code/scripts/tests/test_upgrade_branches.py b/lib/lp/code/scripts/tests/test_upgrade_branches.py
index a8ec74a..9c51d60 100644
--- a/lib/lp/code/scripts/tests/test_upgrade_branches.py
+++ b/lib/lp/code/scripts/tests/test_upgrade_branches.py
@@ -3,15 +3,17 @@
"""Test the upgrade_branches script."""
+import os.path
import transaction
from breezy.branch import Branch as BzrBranch
from lp.code.model.branch import BranchFormat, RepositoryFormat
from lp.code.model.branchjob import BranchUpgradeJob
-from lp.services.scripts.tests import run_script
+from lp.services.config import config
from lp.testing import TestCaseWithFactory
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
class TestUpgradeBranches(TestCaseWithFactory):
@@ -34,10 +36,10 @@ class TestUpgradeBranches(TestCaseWithFactory):
transaction.commit()
retcode, stdout, stderr = run_script(
- "cronscripts/process-job-source.py",
- ["IBranchUpgradeJobSource"],
- expect_returncode=0,
+ os.path.join(config.root, "cronscripts", "process-job-source.py"),
+ args=["IBranchUpgradeJobSource"],
)
+ self.assertEqual(0, retcode)
self.assertEqual("", stdout)
self.assertIn("INFO Ran 1 BranchUpgradeJob jobs.\n", stderr)
@@ -66,10 +68,10 @@ class TestUpgradeBranches(TestCaseWithFactory):
transaction.commit()
retcode, stdout, stderr = run_script(
- "cronscripts/process-job-source.py",
- ["IBranchUpgradeJobSource"],
- expect_returncode=0,
+ os.path.join(config.root, "cronscripts", "process-job-source.py"),
+ args=["IBranchUpgradeJobSource"],
)
+ self.assertEqual(0, retcode)
self.assertEqual("", stdout)
self.assertIn("INFO Ran 1 BranchUpgradeJob jobs.\n", stderr)
diff --git a/lib/lp/registry/tests/test_membership_notification_job.py b/lib/lp/registry/tests/test_membership_notification_job.py
index 9becfe6..9b41096 100644
--- a/lib/lp/registry/tests/test_membership_notification_job.py
+++ b/lib/lp/registry/tests/test_membership_notification_job.py
@@ -3,8 +3,6 @@
"""Tests of `MembershipNotificationJob`."""
-import os
-
import transaction
from testtools.content import text_content
from zope.component import getUtility
@@ -114,12 +112,10 @@ class MembershipNotificationJobTest(TestCaseWithFactory):
)
job_repr = repr(job)
transaction.commit()
- env = os.environ.copy()
- env["LP_DEBUG_SQL"] = "1"
exit_code, out, err = run_script(
"cronscripts/process-job-source.py",
args=["-vv", IMembershipNotificationJobSource.getName()],
- env=env,
+ extra_env={"LP_DEBUG_SQL": "1"},
)
self.addDetail("stdout", text_content(out))
self.addDetail("stderr", text_content(err))
diff --git a/lib/lp/registry/tests/test_person_merge_job.py b/lib/lp/registry/tests/test_person_merge_job.py
index c536471..e317245 100644
--- a/lib/lp/registry/tests/test_person_merge_job.py
+++ b/lib/lp/registry/tests/test_person_merge_job.py
@@ -3,8 +3,6 @@
"""Tests of `PersonMergeJob`."""
-import os
-
import transaction
from testtools.content import text_content
from zope.component import getUtility
@@ -144,12 +142,10 @@ class TestPersonMergeJob(TestCaseWithFactory):
)
transaction.commit()
- env = os.environ.copy()
- env["LP_DEBUG_SQL"] = "1"
exit_code, out, err = run_script(
"cronscripts/process-job-source.py",
args=["-vv", IPersonMergeJobSource.getName()],
- env=env,
+ extra_env={"LP_DEBUG_SQL": "1"},
)
self.addDetail("stdout", text_content(out))
diff --git a/lib/lp/registry/tests/test_productjob.py b/lib/lp/registry/tests/test_productjob.py
index bec594c..982cb08 100644
--- a/lib/lp/registry/tests/test_productjob.py
+++ b/lib/lp/registry/tests/test_productjob.py
@@ -3,7 +3,6 @@
"""Tests for ProductJobs."""
-import os
from datetime import datetime, timedelta, timezone
import transaction
@@ -623,12 +622,10 @@ class CommericialExpirationMixin(CommercialHelpers):
proprietary_job = self.JOB_CLASS.create(proprietary_product, reviewer)
transaction.commit()
- env = os.environ.copy()
- env["LP_DEBUG_SQL"] = "1"
exit_code, out, err = run_script(
"cronscripts/process-job-source.py",
args=["-vv", self.JOB_SOURCE_INTERFACE.getName()],
- env=env,
+ extra_env={"LP_DEBUG_SQL": "1"},
)
self.addDetail("stdout", text_content(out))
self.addDetail("stderr", text_content(err))
diff --git a/lib/lp/registry/tests/test_sharingjob.py b/lib/lp/registry/tests/test_sharingjob.py
index a088b02..6710772 100644
--- a/lib/lp/registry/tests/test_sharingjob.py
+++ b/lib/lp/registry/tests/test_sharingjob.py
@@ -3,8 +3,6 @@
"""Tests for SharingJobs."""
-import os
-
import transaction
from testtools.content import text_content
from zope.component import getUtility
@@ -264,12 +262,10 @@ class TestRunViaCron(TestCaseWithFactory):
)
transaction.commit()
- env = os.environ.copy()
- env["LP_DEBUG_SQL"] = "1"
exit_code, out, err = run_script(
"cronscripts/process-job-source.py",
args=["-vv", job_type],
- env=env,
+ extra_env={"LP_DEBUG_SQL": "1"},
)
self.addDetail("stdout", text_content(out))
self.addDetail("stderr", text_content(err))
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index cd06fcc..61d29ff 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -105,7 +105,6 @@ from lp.services.librarian.model import TimeLimitedToken
from lp.services.messages.interfaces.message import IMessageSet
from lp.services.messages.model.message import Message
from lp.services.openid.model.openidconsumer import OpenIDConsumerNonce
-from lp.services.scripts.tests import run_script
from lp.services.session.model import SessionData, SessionPkgData
from lp.services.verification.interfaces.authtoken import LoginTokenType
from lp.services.verification.model.logintoken import LoginToken
@@ -144,6 +143,7 @@ from lp.testing.layers import (
ZopelessDatabaseLayer,
)
from lp.testing.mail_helpers import pop_notifications
+from lp.testing.script import run_script
from lp.translations.model.pofile import POFile
from lp.translations.model.potmsgset import POTMsgSet
from lp.translations.model.translationtemplateitem import (
@@ -158,18 +158,16 @@ class TestGarboScript(TestCase):
def test_daily_script(self):
"""Ensure garbo-daily.py actually runs."""
- rv, out, err = run_script(
- "cronscripts/garbo-daily.py", ["-q"], expect_returncode=0
- )
+ rv, out, err = run_script("cronscripts/garbo-daily.py", args=["-q"])
+ self.assertEqual(0, rv)
self.assertFalse(out.strip(), "Output to stdout: %s" % out)
self.assertFalse(err.strip(), "Output to stderr: %s" % err)
DatabaseLayer.force_dirty_database()
def test_hourly_script(self):
"""Ensure garbo-hourly.py actually runs."""
- rv, out, err = run_script(
- "cronscripts/garbo-hourly.py", ["-q"], expect_returncode=0
- )
+ rv, out, err = run_script("cronscripts/garbo-hourly.py", args=["-q"])
+ self.assertEqual(0, rv)
self.assertFalse(out.strip(), "Output to stdout: %s" % out)
self.assertFalse(err.strip(), "Output to stderr: %s" % err)
DatabaseLayer.force_dirty_database()
diff --git a/lib/lp/services/job/scripts/tests/test_process_job_source.py b/lib/lp/services/job/scripts/tests/test_process_job_source.py
index b66e847..6b0465e 100644
--- a/lib/lp/services/job/scripts/tests/test_process_job_source.py
+++ b/lib/lp/services/job/scripts/tests/test_process_job_source.py
@@ -17,10 +17,10 @@ from lp.registry.interfaces.teammembership import (
from lp.services.config import config
from lp.services.job.scripts import process_job_source
from lp.services.scripts.base import LOCK_PATH
-from lp.services.scripts.tests import run_script
from lp.testing import TestCase, TestCaseWithFactory, login_person
from lp.testing.layers import LaunchpadScriptLayer
from lp.testing.matchers import DocTestMatches
+from lp.testing.script import run_script
class ProcessSingleJobSourceConfigTest(TestCase):
@@ -55,9 +55,8 @@ class ProcessJobSourceTest(TestCaseWithFactory):
def test_missing_argument(self):
# The script should display usage info when called without any
# arguments.
- returncode, output, error = run_script(
- self.script, [], expect_returncode=1
- )
+ returncode, output, error = run_script(self.script)
+ self.assertEqual(1, returncode)
self.assertIn("Usage:", output)
self.assertIn("process-job-source.py [options] JOB_SOURCE", output)
@@ -65,8 +64,9 @@ class ProcessJobSourceTest(TestCaseWithFactory):
# The script should just create a lockfile and exit if no jobs
# are in the queue.
returncode, output, error = run_script(
- self.script, ["IMembershipNotificationJobSource"]
+ self.script, args=["IMembershipNotificationJobSource"]
)
+ self.assertEqual(0, returncode)
expected = (
"INFO Creating lockfile: .*launchpad-process-job-"
"source-IMembershipNotificationJobSource.lock.*"
@@ -85,10 +85,9 @@ class ProcessJobSourceTest(TestCaseWithFactory):
lock.acquire()
try:
returncode, output, error = run_script(
- self.script,
- ["IMembershipNotificationJobSource"],
- expect_returncode=1,
+ self.script, args=["IMembershipNotificationJobSource"]
)
+ self.assertEqual(1, returncode)
expected = dedent(
"""\
INFO Creating lockfile: {lock}
@@ -111,8 +110,9 @@ class ProcessJobSourceTest(TestCaseWithFactory):
tm.setStatus(TeamMembershipStatus.ADMIN, team.teamowner)
transaction.commit()
returncode, output, error = run_script(
- self.script, ["-v", "IMembershipNotificationJobSource"]
+ self.script, args=["-v", "IMembershipNotificationJobSource"]
)
+ self.assertEqual(0, returncode)
self.assertIn(
(
"INFO Running <MembershipNotificationJob "
@@ -149,9 +149,8 @@ class ProcessJobSourceGroupsTest(TestCaseWithFactory):
def test_missing_argument(self):
# The script should display usage info when called without any
# arguments.
- returncode, output, error = run_script(
- self.script, [], expect_returncode=1
- )
+ returncode, output, error = run_script(self.script)
+ self.assertEqual(1, returncode)
self.assertIn(
(
"Usage: process-job-source-groups.py "
@@ -167,7 +166,8 @@ class ProcessJobSourceGroupsTest(TestCaseWithFactory):
# The script should just run over each job source class, and then
# exit if no jobs are in the queue. It should not create its own
# lockfile.
- returncode, output, error = run_script(self.script, ["MAIN"])
+ returncode, output, error = run_script(self.script, args=["MAIN"])
+ self.assertEqual(0, returncode)
expected = (
".*Creating lockfile:.*launchpad-process-job-"
"source-IMembershipNotificationJobSource.lock.*"
@@ -186,7 +186,10 @@ class ProcessJobSourceGroupsTest(TestCaseWithFactory):
tm = membership_set.getByPersonAndTeam(person, team)
tm.setStatus(TeamMembershipStatus.ADMIN, team.teamowner)
transaction.commit()
- returncode, output, error = run_script(self.script, ["-v", "MAIN"])
+ returncode, output, error = run_script(
+ self.script, args=["-v", "MAIN"]
+ )
+ self.assertEqual(0, returncode)
self.assertTextMatchesExpressionIgnoreWhitespace(
(
"INFO Running <MembershipNotificationJob "
@@ -202,7 +205,8 @@ class ProcessJobSourceGroupsTest(TestCaseWithFactory):
args = ["MAIN"]
for source in self.getJobSources("MAIN"):
args.extend(("--exclude", source))
- returncode, output, error = run_script(self.script, args)
+ returncode, output, error = run_script(self.script, args=args)
+ self.assertEqual(0, returncode)
self.assertEqual("", error)
def test_exclude_non_existing_group(self):
@@ -212,6 +216,7 @@ class ProcessJobSourceGroupsTest(TestCaseWithFactory):
for source in self.getJobSources("MAIN"):
args.extend(("--exclude", source))
args.extend(("--exclude", "BobbyDazzler"))
- returncode, output, error = run_script(self.script, args)
+ returncode, output, error = run_script(self.script, args=args)
+ self.assertEqual(0, returncode)
expected = "INFO 'BobbyDazzler' is not in MAIN\n"
self.assertThat(error, DocTestMatches(expected))
diff --git a/lib/lp/services/scripts/tests/__init__.py b/lib/lp/services/scripts/tests/__init__.py
index 923eff1..7bcea71 100644
--- a/lib/lp/services/scripts/tests/__init__.py
+++ b/lib/lp/services/scripts/tests/__init__.py
@@ -7,10 +7,8 @@ __all__ = [
import os
-import subprocess
import lp
-from lp.services.config import config
LP_TREE = os.path.dirname(os.path.dirname(os.path.dirname(lp.__file__)))
@@ -37,40 +35,3 @@ def find_lp_scripts():
continue
scripts.append(script_path)
return sorted(scripts)
-
-
-def run_script(
- script_relpath,
- args,
- expect_returncode=0,
- extra_env=None,
- universal_newlines=True,
-):
- """Run a script for testing purposes.
-
- :param script_relpath: The relative path to the script, from the tree
- root.
- :param args: Arguments to provide to the script.
- :param expect_returncode: The return code expected. If a different value
- is returned, and exception will be raised.
- :param extra_env: A dictionary of extra environment variables to provide
- to the script, or None.
- :param universal_newlines: Passed to `subprocess.Popen`, defaulting to
- True.
- """
- script = os.path.join(config.root, script_relpath)
- args = [script] + args
- env = dict(os.environ)
- if extra_env is not None:
- env.update(extra_env)
- process = subprocess.Popen(
- args,
- stdout=subprocess.PIPE,
- stderr=subprocess.PIPE,
- env=env,
- universal_newlines=universal_newlines,
- )
- stdout, stderr = process.communicate()
- if process.returncode != expect_returncode:
- raise AssertionError("Failed:\n%s\n%s" % (stdout, stderr))
- return (process.returncode, stdout, stderr)
diff --git a/lib/lp/services/webhooks/tests/test_job.py b/lib/lp/services/webhooks/tests/test_job.py
index 46e07e6..1e30e53 100644
--- a/lib/lp/services/webhooks/tests/test_job.py
+++ b/lib/lp/services/webhooks/tests/test_job.py
@@ -42,7 +42,6 @@ from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.runner import JobRunner
from lp.services.job.tests import block_on_job
-from lp.services.scripts.tests import run_script
from lp.services.webhooks.client import WebhookClient, create_request
from lp.services.webhooks.interfaces import (
IWebhookClient,
@@ -69,6 +68,7 @@ from lp.testing.layers import (
DatabaseFunctionalLayer,
ZopelessDatabaseLayer,
)
+from lp.testing.script import run_script
class TestWebhookJob(TestCaseWithFactory):
@@ -927,9 +927,9 @@ class TestViaCronscript(TestCaseWithFactory):
retcode, stdout, stderr = run_script(
"cronscripts/process-job-source.py",
- ["IWebhookDeliveryJobSource"],
- expect_returncode=0,
+ args=["IWebhookDeliveryJobSource"],
)
+ self.assertEqual(0, retcode)
self.assertEqual("", stdout)
self.assertIn(
"INFO Scheduling retry due to WebhookDeliveryRetry", stderr
diff --git a/lib/lp/soyuz/tests/test_distroseriesdifferencejob.py b/lib/lp/soyuz/tests/test_distroseriesdifferencejob.py
index b0a2e63..085ae81 100644
--- a/lib/lp/soyuz/tests/test_distroseriesdifferencejob.py
+++ b/lib/lp/soyuz/tests/test_distroseriesdifferencejob.py
@@ -21,7 +21,6 @@ from lp.services.database.interfaces import IPrimaryStore
from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.tests import block_on_job
-from lp.services.scripts.tests import run_script
from lp.soyuz.enums import ArchivePurpose, PackagePublishingStatus
from lp.soyuz.interfaces.distributionjob import (
DistributionJobType,
@@ -43,6 +42,7 @@ from lp.testing.layers import (
LaunchpadZopelessLayer,
ZopelessDatabaseLayer,
)
+from lp.testing.script import run_script
def find_dsd_for(dsp, package):
@@ -573,10 +573,10 @@ class TestDistroSeriesDifferenceJobSource(TestCaseWithFactory):
transaction.commit()
return_code, stdout, stderr = run_script(
"cronscripts/process-job-source.py",
- ["-v", "IDistroSeriesDifferenceJobSource"],
+ args=["-v", "IDistroSeriesDifferenceJobSource"],
)
# The cronscript ran how we expected it to.
- self.assertEqual(return_code, 0)
+ self.assertEqual(0, return_code)
self.assertIn("INFO Ran 1 DistroSeriesDifferenceJob jobs.", stderr)
# And it did what we expected.
jobs = find_waiting_jobs(
diff --git a/lib/lp/soyuz/tests/test_initializedistroseriesjob.py b/lib/lp/soyuz/tests/test_initializedistroseriesjob.py
index 4f5b0d7..a49b785 100644
--- a/lib/lp/soyuz/tests/test_initializedistroseriesjob.py
+++ b/lib/lp/soyuz/tests/test_initializedistroseriesjob.py
@@ -14,7 +14,6 @@ from lp.registry.interfaces.distroseriesparent import IDistroSeriesParentSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.features.testing import FeatureFixture
from lp.services.job.tests import block_on_job
-from lp.services.scripts.tests import run_script
from lp.soyuz.enums import SourcePackageFormat
from lp.soyuz.interfaces.distributionjob import (
IInitializeDistroSeriesJobSource,
@@ -37,6 +36,7 @@ from lp.testing.layers import (
DatabaseLayer,
LaunchpadZopelessLayer,
)
+from lp.testing.script import run_script
class InitializeDistroSeriesJobTests(TestCaseWithFactory):
@@ -401,10 +401,11 @@ class InitializeDistroSeriesJobTestsWithPackages(TestCaseWithFactory):
self.assertEqual("amd64", child.nominatedarchindep.architecturetag)
def test_cronscript(self):
- run_script(
+ exit_code, _, _ = run_script(
"cronscripts/process-job-source.py",
- ["IInitializeDistroSeriesJobSource"],
+ args=["IInitializeDistroSeriesJobSource"],
)
+ self.assertEqual(0, exit_code)
DatabaseLayer.force_dirty_database()
diff --git a/lib/lp/soyuz/tests/test_packagecopyjob.py b/lib/lp/soyuz/tests/test_packagecopyjob.py
index c0f3a85..7414c1b 100644
--- a/lib/lp/soyuz/tests/test_packagecopyjob.py
+++ b/lib/lp/soyuz/tests/test_packagecopyjob.py
@@ -726,12 +726,10 @@ class PlainPackageCopyJobTests(TestCaseWithFactory, LocalTestHelper):
archive2.newComponentUploader(requester, "main")
transaction.commit()
- env = os.environ.copy()
- env["LP_DEBUG_SQL"] = "1"
exit_code, out, err = run_script(
"cronscripts/process-job-source.py",
args=["-vv", IPlainPackageCopyJobSource.getName()],
- env=env,
+ extra_env={"LP_DEBUG_SQL": "1"},
)
self.addDetail("stdout", text_content(out))
diff --git a/lib/lp/soyuz/tests/test_packagediffjob.py b/lib/lp/soyuz/tests/test_packagediffjob.py
index 54c5a65..88988c7 100644
--- a/lib/lp/soyuz/tests/test_packagediffjob.py
+++ b/lib/lp/soyuz/tests/test_packagediffjob.py
@@ -1,8 +1,6 @@
# Copyright 2013-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-import os
-
import transaction
from testtools.content import text_content
from zope.component import getUtility
@@ -79,12 +77,10 @@ class TestPackageDiffJob(TestCaseWithFactory):
def test_smoke(self):
diff = create_proper_job(self.factory)
transaction.commit()
- env = os.environ.copy()
- env["LP_DEBUG_SQL"] = "1"
exit_code, out, err = run_script(
"cronscripts/process-job-source.py",
args=["-vv", IPackageDiffJobSource.getName()],
- env=env,
+ extra_env={"LP_DEBUG_SQL": "1"},
)
self.addDetail("stdout", text_content(out))
diff --git a/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py b/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py
index 650f39d..bc6289e 100644
--- a/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py
+++ b/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py
@@ -1,8 +1,6 @@
# Copyright 2013-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-import os
-
import transaction
from testtools.content import text_content
from zope.component import getUtility
@@ -130,12 +128,10 @@ class TestPackageTranslationsUploadJob(LocalTestHelper):
}
spr, sp, job = self.makeJob(tar_content=tar_content)
transaction.commit()
- env = os.environ.copy()
- env["LP_DEBUG_SQL"] = "1"
exit_code, out, err = run_script(
"cronscripts/process-job-source.py",
args=["-vv", IPackageTranslationsUploadJobSource.getName()],
- env=env,
+ extra_env={"LP_DEBUG_SQL": "1"},
)
self.addDetail("stdout", text_content(out))
diff --git a/lib/lp/soyuz/tests/test_processacceptedbugsjob.py b/lib/lp/soyuz/tests/test_processacceptedbugsjob.py
index f2e29bf..b5c9b34 100644
--- a/lib/lp/soyuz/tests/test_processacceptedbugsjob.py
+++ b/lib/lp/soyuz/tests/test_processacceptedbugsjob.py
@@ -4,7 +4,6 @@
"""Tests for jobs to close bugs for accepted package uploads."""
import io
-import os
from itertools import product
from textwrap import dedent
@@ -448,12 +447,10 @@ class TestProcessAcceptedBugsJob(TestCaseWithFactory):
self.makeJob(spr=spr, bug_ids=[bug.id])
transaction.commit()
- env = os.environ.copy()
- env["LP_DEBUG_SQL"] = "1"
exit_code, out, err = run_script(
"cronscripts/process-job-source.py",
args=["-vv", IProcessAcceptedBugsJobSource.getName()],
- env=env,
+ extra_env={"LP_DEBUG_SQL": "1"},
)
self.addDetail("stdout", text_content(out))
diff --git a/lib/lp/testing/script.py b/lib/lp/testing/script.py
index 4f4e5e9..039bd60 100644
--- a/lib/lp/testing/script.py
+++ b/lib/lp/testing/script.py
@@ -61,7 +61,7 @@ def run_command(
def run_script(
script: str,
args: Optional[List[str]] = None,
- env: Optional[Dict[str, str]] = None,
+ extra_env: Optional[Dict[str, str]] = None,
cwd: Optional[str] = None,
input: Optional[Union[bytes, str]] = None,
universal_newlines: bool = True,
@@ -70,19 +70,20 @@ def run_script(
:param script: Python script to run.
:param args: optional list of command-line arguments.
- :param env: optional environment dict; if none is given, the script will
- get a copy of the environment of the calling process. In either
- case, `PYTHONPATH` is removed since inheriting it may break some
- scripts.
- :param env: optional, passed to `subprocess.Popen`.
+ :param extra_env: optional dict of extra environment variables to pass
+ to the script, in addition to those in the environment of the
+ calling process. Regardless of whether this is passed, the
+ `PYTHONPATH` environment variable is removed since inheriting it may
+ break some scripts.
:param cwd: optional, passed to `subprocess.Popen`.
:param input: optional string to feed to standard input.
:param universal_newlines: passed to `subprocess.Popen`, defaulting to
True.
:return: tuple of return value, standard output, and standard error.
"""
- if env is None:
- env = os.environ.copy()
+ env = os.environ.copy()
+ if extra_env is not None:
+ env.update(extra_env)
env.pop("PYTHONPATH", None)
return run_command(
diff --git a/lib/lp/translations/doc/message-sharing-merge-script.rst b/lib/lp/translations/doc/message-sharing-merge-script.rst
index 09aa659..441c4c9 100644
--- a/lib/lp/translations/doc/message-sharing-merge-script.rst
+++ b/lib/lp/translations/doc/message-sharing-merge-script.rst
@@ -7,10 +7,10 @@ translations into properly sharing ones.
Unit tests go through the details of how the script works. Here we just
show that the script can run and perform its work.
- >>> from lp.services.scripts.tests import run_script
+ >>> from lp.testing.script import run_script
>>> retcode, stdout, stderr = run_script(
... "scripts/rosetta/message-sharing-merge.py",
- ... ["-q", "-P", "-T", "-d", "ubuntu", "-s", "evolution"],
+ ... args=["-q", "-P", "-T", "-d", "ubuntu", "-s", "evolution"],
... )
The migration succeeds.
diff --git a/lib/lp/translations/doc/translations-export-to-branch.rst b/lib/lp/translations/doc/translations-export-to-branch.rst
index f01c7bb..df55b23 100644
--- a/lib/lp/translations/doc/translations-export-to-branch.rst
+++ b/lib/lp/translations/doc/translations-export-to-branch.rst
@@ -5,9 +5,9 @@ The translations-export-to-branch script visits all ProductSeries with a
translations_branch set, and for each, exports the series' translations
to that branch.
- >>> from lp.services.scripts.tests import run_script
+ >>> from lp.testing.script import run_script
>>> ret_code, stdout, stderr = run_script(
- ... "cronscripts/translations-export-to-branch.py", []
+ ... "cronscripts/translations-export-to-branch.py"
... )
>>> ret_code
0
diff --git a/lib/lp/translations/pottery/tests/test_detect_intltool.py b/lib/lp/translations/pottery/tests/test_detect_intltool.py
index c25078f..c44af7f 100644
--- a/lib/lp/translations/pottery/tests/test_detect_intltool.py
+++ b/lib/lp/translations/pottery/tests/test_detect_intltool.py
@@ -7,8 +7,9 @@ from textwrap import dedent
from breezy.controldir import ControlDir
-from lp.services.scripts.tests import run_script
+from lp.services.config import config
from lp.testing import TestCase
+from lp.testing.script import run_script
from lp.translations.pottery.detect_intltool import is_intltool_structure
@@ -58,9 +59,15 @@ class SetupTestPackageMixin:
self.prepare_package("intltool_POTFILES_in_2")
return_code, stdout, stderr = run_script(
- "scripts/rosetta/pottery-generate-intltool.py", []
+ os.path.join(
+ config.root,
+ "scripts",
+ "rosetta",
+ "pottery-generate-intltool.py",
+ )
)
+ self.assertEqual(0, return_code)
self.assertEqual(
dedent(
"""\
diff --git a/lib/lp/translations/scripts/tests/test_merge_existing_packagings.py b/lib/lp/translations/scripts/tests/test_merge_existing_packagings.py
index 22e3438..97d2334 100644
--- a/lib/lp/translations/scripts/tests/test_merge_existing_packagings.py
+++ b/lib/lp/translations/scripts/tests/test_merge_existing_packagings.py
@@ -6,9 +6,9 @@
import transaction
-from lp.services.scripts.tests import run_script
from lp.testing import TestCaseWithFactory, person_logged_in
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
from lp.translations.tests.test_translationpackagingjob import (
count_translations,
make_translation_merge_job,
@@ -30,10 +30,9 @@ class TestMergeExistingPackagings(TestCaseWithFactory):
self.assertEqual(2, count_translations(job))
transaction.commit()
retcode, stdout, stderr = run_script(
- "scripts/rosetta/merge-existing-packagings.py",
- [],
- expect_returncode=0,
+ "scripts/rosetta/merge-existing-packagings.py"
)
+ self.assertEqual(0, retcode)
merge_message = "INFO Merging %s/%s and %s/%s.\n" % (
job.productseries.product.name,
job.productseries.name,
diff --git a/lib/lp/translations/scripts/tests/test_packaging_translations.py b/lib/lp/translations/scripts/tests/test_packaging_translations.py
index 5c9ece4..ea40476 100644
--- a/lib/lp/translations/scripts/tests/test_packaging_translations.py
+++ b/lib/lp/translations/scripts/tests/test_packaging_translations.py
@@ -9,9 +9,9 @@ from textwrap import dedent
import transaction
from testtools.matchers import MatchesRegex
-from lp.services.scripts.tests import run_script
from lp.testing import TestCaseWithFactory, admin_logged_in
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
from lp.translations.tests.test_translationpackagingjob import (
make_translation_merge_job,
)
@@ -26,9 +26,9 @@ class TestMergeTranslations(TestCaseWithFactory):
transaction.commit()
retcode, stdout, stderr = run_script(
"cronscripts/process-job-source.py",
- ["ITranslationPackagingJobSource"],
- expect_returncode=0,
+ args=["ITranslationPackagingJobSource"],
)
+ self.assertEqual(0, retcode)
matcher = MatchesRegex(
dedent(
"""\
@@ -53,9 +53,9 @@ class TestMergeTranslations(TestCaseWithFactory):
transaction.commit()
retcode, stdout, stderr = run_script(
"cronscripts/process-job-source.py",
- ["ITranslationPackagingJobSource"],
- expect_returncode=0,
+ args=["ITranslationPackagingJobSource"],
)
+ self.assertEqual(0, retcode)
matcher = MatchesRegex(
dedent(
"""\
diff --git a/lib/lp/translations/scripts/tests/test_reupload_translations.py b/lib/lp/translations/scripts/tests/test_reupload_translations.py
index 913889a..bb1cdf5 100644
--- a/lib/lp/translations/scripts/tests/test_reupload_translations.py
+++ b/lib/lp/translations/scripts/tests/test_reupload_translations.py
@@ -14,12 +14,12 @@ from zope.security.proxy import removeSecurityProxy
from lp.registry.model.sourcepackage import SourcePackage
from lp.services.librarian.model import LibraryFileAliasSet
-from lp.services.scripts.tests import run_script
from lp.soyuz.model.packagetranslationsuploadjob import (
_filter_ubuntu_translation_file,
)
from lp.testing import TestCaseWithFactory
from lp.testing.layers import LaunchpadZopelessLayer
+from lp.testing.script import run_script
from lp.translations.model.translationimportqueue import TranslationImportQueue
from lp.translations.scripts.reupload_translations import (
ReuploadPackageTranslations,
@@ -193,7 +193,7 @@ class TestReuploadScript(TestCaseWithFactory):
"""Test a run of the script."""
retcode, stdout, stderr = run_script(
"scripts/rosetta/reupload-translations.py",
- [
+ args=[
"-d",
self.distroseries.distribution.name,
"-s",
diff --git a/lib/lp/translations/scripts/tests/test_translations_to_branch.py b/lib/lp/translations/scripts/tests/test_translations_to_branch.py
index fcc38a7..ac57417 100644
--- a/lib/lp/translations/scripts/tests/test_translations_to_branch.py
+++ b/lib/lp/translations/scripts/tests/test_translations_to_branch.py
@@ -3,6 +3,7 @@
"""Acceptance test for the translations-export-to-branch script."""
+import os.path
import re
from datetime import datetime, timedelta, timezone
from textwrap import dedent
@@ -22,10 +23,10 @@ from lp.registry.model.productseries import ProductSeries
from lp.services.config import config
from lp.services.database.interfaces import IStandbyStore
from lp.services.log.logger import BufferLogger
-from lp.services.scripts.tests import run_script
from lp.testing import TestCaseWithFactory, map_branch_contents
from lp.testing.fakemethod import FakeMethod
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
from lp.translations.scripts.translations_to_branch import (
ExportTranslationsToBranch,
)
@@ -93,7 +94,10 @@ class TestExportTranslationsToBranch(TestCaseWithFactory):
# Run The Script.
retcode, stdout, stderr = run_script(
- "cronscripts/translations-export-to-branch.py", ["-vvv"]
+ os.path.join(
+ config.root, "cronscripts", "translations-export-to-branch.py"
+ ),
+ args=["-vvv"],
)
self.assertEqual("", stdout)
@@ -145,8 +149,10 @@ class TestExportTranslationsToBranch(TestCaseWithFactory):
# anything because it sees that the POFile has not been changed
# since the last export.
retcode, stdout, stderr = run_script(
- "cronscripts/translations-export-to-branch.py",
- ["-vvv", "--no-fudge"],
+ os.path.join(
+ config.root, "cronscripts", "translations-export-to-branch.py"
+ ),
+ args=["-vvv", "--no-fudge"],
)
self.assertEqual(0, retcode)
self.assertIn("Last commit was at", stderr)
diff --git a/lib/lp/translations/tests/test_rosetta_branches_script.py b/lib/lp/translations/tests/test_rosetta_branches_script.py
index d4c3ede..4480a44 100644
--- a/lib/lp/translations/tests/test_rosetta_branches_script.py
+++ b/lib/lp/translations/tests/test_rosetta_branches_script.py
@@ -7,14 +7,17 @@ This would normally be done in a doctest but TestCaseWithFactory has all the
provisions to handle Bazaar branches.
"""
+import os.path
+
import transaction
from zope.component import getUtility
from lp.code.model.branchjob import RosettaUploadJob
+from lp.services.config import config
from lp.services.osutils import override_environ
-from lp.services.scripts.tests import run_script
from lp.testing import TestCaseWithFactory
from lp.testing.layers import ZopelessAppServerLayer
+from lp.testing.script import run_script
from lp.translations.enums import RosettaImportStatus
from lp.translations.interfaces.translationimportqueue import (
ITranslationImportQueue,
@@ -65,7 +68,8 @@ class TestRosettaBranchesScript(TestCaseWithFactory):
transaction.commit()
return_code, stdout, stderr = run_script(
- "cronscripts/process-job-source.py", ["IRosettaUploadJobSource"]
+ os.path.join(config.root, "cronscripts", "process-job-source.py"),
+ args=["IRosettaUploadJobSource"],
)
self.assertEqual(0, return_code)
