diff --git a/lib/spack/spack/ci/__init__.py b/lib/spack/spack/ci/__init__.py index aff09983a82..faeb66965d9 100644 --- a/lib/spack/spack/ci/__init__.py +++ b/lib/spack/spack/ci/__init__.py @@ -14,7 +14,7 @@ import zipfile from collections import namedtuple from typing import Callable, Dict, List, Set -from urllib.request import HTTPHandler, Request, build_opener +from urllib.request import Request import llnl.util.filesystem as fs import llnl.util.tty as tty @@ -62,6 +62,8 @@ PushResult = namedtuple("PushResult", "success url") +urlopen = web_util.urlopen # alias for mocking in tests + def get_change_revisions(): """If this is a git repo get the revisions to use when checking @@ -627,29 +629,19 @@ def download_and_extract_artifacts(url, work_dir): if token: headers["PRIVATE-TOKEN"] = token - opener = build_opener(HTTPHandler) - - request = Request(url, headers=headers) - request.get_method = lambda: "GET" - - response = opener.open(request, timeout=SPACK_CDASH_TIMEOUT) - response_code = response.getcode() - - if response_code != 200: - msg = f"Error response code ({response_code}) in reproduce_ci_job" - raise SpackError(msg) - + request = Request(url, headers=headers, method="GET") artifacts_zip_path = os.path.join(work_dir, "artifacts.zip") + os.makedirs(work_dir, exist_ok=True) - if not os.path.exists(work_dir): - os.makedirs(work_dir) + try: + response = urlopen(request, timeout=SPACK_CDASH_TIMEOUT) + with open(artifacts_zip_path, "wb") as out_file: + shutil.copyfileobj(response, out_file) + except OSError as e: + raise SpackError(f"Error fetching artifacts: {e}") - with open(artifacts_zip_path, "wb") as out_file: - shutil.copyfileobj(response, out_file) - - zip_file = zipfile.ZipFile(artifacts_zip_path) - zip_file.extractall(work_dir) - zip_file.close() + with zipfile.ZipFile(artifacts_zip_path) as zip_file: + zip_file.extractall(work_dir) os.remove(artifacts_zip_path) diff --git a/lib/spack/spack/reporters/cdash.py b/lib/spack/spack/reporters/cdash.py index 4368aa5dd09..045236f66ff 100644 --- a/lib/spack/spack/reporters/cdash.py +++ b/lib/spack/spack/reporters/cdash.py @@ -1,6 +1,7 @@ # Copyright Spack Project Developers. See COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import codecs import collections import hashlib import os @@ -13,7 +14,7 @@ import xml.sax.saxutils from typing import Dict, Optional from urllib.parse import urlencode -from urllib.request import HTTPSHandler, Request, build_opener +from urllib.request import Request import llnl.util.tty as tty from llnl.util.filesystem import working_dir @@ -24,10 +25,10 @@ import spack.spec import spack.tengine import spack.util.git +import spack.util.web as web_util from spack.error import SpackError from spack.util.crypto import checksum from spack.util.log_parse import parse_log_events -from spack.util.web import ssl_create_default_context from .base import Reporter from .extract import extract_test_parts @@ -433,7 +434,6 @@ def upload(self, filename): # Compute md5 checksum for the contents of this file. md5sum = checksum(hashlib.md5, filename, block_size=8192) - opener = build_opener(HTTPSHandler(context=ssl_create_default_context())) with open(filename, "rb") as f: params_dict = { "build": self.buildname, @@ -443,26 +443,21 @@ def upload(self, filename): } encoded_params = urlencode(params_dict) url = "{0}&{1}".format(self.cdash_upload_url, encoded_params) - request = Request(url, data=f) + request = Request(url, data=f, method="PUT") request.add_header("Content-Type", "text/xml") request.add_header("Content-Length", os.path.getsize(filename)) if self.authtoken: request.add_header("Authorization", "Bearer {0}".format(self.authtoken)) try: - # By default, urllib2 only support GET and POST. - # CDash expects this file to be uploaded via PUT. - request.get_method = lambda: "PUT" - response = opener.open(request, timeout=SPACK_CDASH_TIMEOUT) + response = web_util.urlopen(request, timeout=SPACK_CDASH_TIMEOUT) if self.current_package_name not in self.buildIds: - resp_value = response.read() - if isinstance(resp_value, bytes): - resp_value = resp_value.decode("utf-8") + resp_value = codecs.getreader("utf-8")(response).read() match = self.buildid_regexp.search(resp_value) if match: buildid = match.group(1) self.buildIds[self.current_package_name] = buildid except Exception as e: - print("Upload to CDash failed: {0}".format(e)) + print(f"Upload to CDash failed: {e}") def finalize_report(self): if self.buildIds: diff --git a/lib/spack/spack/test/ci.py b/lib/spack/spack/test/ci.py index 9d00825ec3d..2e67709696b 100644 --- a/lib/spack/spack/test/ci.py +++ b/lib/spack/spack/test/ci.py @@ -1,8 +1,10 @@ # Copyright Spack Project Developers. See COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import io import os import subprocess +from urllib.error import HTTPError import pytest @@ -15,6 +17,7 @@ import spack.paths as spack_paths import spack.repo as repo import spack.util.git +from spack.test.conftest import MockHTTPResponse pytestmark = [pytest.mark.usefixtures("mock_packages")] @@ -162,38 +165,8 @@ def test_import_signing_key(mock_gnupghome): ci.import_signing_key(signing_key) -class FakeWebResponder: - def __init__(self, response_code=200, content_to_read=[]): - self._resp_code = response_code - self._content = content_to_read - self._read = [False for c in content_to_read] - - def open(self, request, data=None, timeout=object()): - return self - - def getcode(self): - return self._resp_code - - def read(self, length=None): - if len(self._content) <= 0: - return None - - if not self._read[-1]: - return_content = self._content[-1] - if length: - self._read[-1] = True - else: - self._read.pop() - self._content.pop() - return return_content - - self._read.pop() - self._content.pop() - return None - - -def test_download_and_extract_artifacts(tmpdir, monkeypatch, working_env): - os.environ.update({"GITLAB_PRIVATE_TOKEN": "faketoken"}) +def test_download_and_extract_artifacts(tmpdir, monkeypatch): + monkeypatch.setenv("GITLAB_PRIVATE_TOKEN", "faketoken") url = "https://www.nosuchurlexists.itsfake/artifacts.zip" working_dir = os.path.join(tmpdir.strpath, "repro") @@ -201,10 +174,13 @@ def test_download_and_extract_artifacts(tmpdir, monkeypatch, working_env): spack_paths.test_path, "data", "ci", "gitlab", "artifacts.zip" ) - with open(test_artifacts_path, "rb") as fd: - fake_responder = FakeWebResponder(content_to_read=[fd.read()]) + def _urlopen_OK(*args, **kwargs): + with open(test_artifacts_path, "rb") as f: + return MockHTTPResponse( + "200", "OK", {"Content-Type": "application/zip"}, io.BytesIO(f.read()) + ) - monkeypatch.setattr(ci, "build_opener", lambda handler: fake_responder) + monkeypatch.setattr(ci, "urlopen", _urlopen_OK) ci.download_and_extract_artifacts(url, working_dir) @@ -214,7 +190,11 @@ def test_download_and_extract_artifacts(tmpdir, monkeypatch, working_env): found_install = fs.find(working_dir, "install.sh") assert len(found_install) == 1 - fake_responder._resp_code = 400 + def _urlopen_500(*args, **kwargs): + raise HTTPError(url, 500, "Internal Server Error", {}, None) + + monkeypatch.setattr(ci, "urlopen", _urlopen_500) + with pytest.raises(spack.error.SpackError): ci.download_and_extract_artifacts(url, working_dir)