diff --git a/dev/build/Dockerfile b/dev/build/Dockerfile
index d13405fd4ba..7f6c7453b27 100644
--- a/dev/build/Dockerfile
+++ b/dev/build/Dockerfile
@@ -25,6 +25,14 @@ RUN pip3 --disable-pip-version-check --no-cache-dir install -r requirements.txt
ietf/manage.py patch_libraries && \
rm -f ietf/settings_local.py
+# Install idnits3. The version is pinned here and in the other container
+# definitions - grep for @ietf-tools/idnits to bump them together.
+# It goes in its own npm prefix so that it can never disturb the idnits2 script
+# at /usr/local/bin/idnits that comes from the base image.
+RUN npm install -g --prefix /usr/local/idnits3 @ietf-tools/idnits@3.1.0 && \
+ ln -sf /usr/local/idnits3/bin/idnits /usr/local/bin/idnits3 && \
+ idnits3 --version
+
RUN chmod +x start.sh && \
chmod +x datatracker-start.sh && \
chmod +x migration-start.sh && \
diff --git a/dev/diff/settings_local.py b/dev/diff/settings_local.py
index c255cac23dc..e9814a46377 100644
--- a/dev/diff/settings_local.py
+++ b/dev/diff/settings_local.py
@@ -18,6 +18,7 @@
IDSUBMIT_IDNITS_BINARY = "/usr/local/bin/idnits"
+IDSUBMIT_IDNITS3_BINARY = "/usr/local/bin/idnits3"
IDSUBMIT_REPOSITORY_PATH = "test/id/"
IDSUBMIT_STAGING_PATH = "test/staging/"
diff --git a/dev/tests/prepare.sh b/dev/tests/prepare.sh
index 47917e45449..8b1e8cbd679 100644
--- a/dev/tests/prepare.sh
+++ b/dev/tests/prepare.sh
@@ -8,6 +8,10 @@ echo "Copying config files..."
cp ./dev/tests/settings_local.py ./ietf/settings_local.py
echo "Ensure all requirements.txt packages are installed..."
pip --disable-pip-version-check --no-cache-dir install -r requirements.txt
+echo "Installing idnits3..."
+npm install -g --prefix /usr/local/idnits3 @ietf-tools/idnits@3.1.0
+ln -sf /usr/local/idnits3/bin/idnits /usr/local/bin/idnits3
+idnits3 --version
echo "Compiling native node packages..."
npm ci
echo "Building static assets..."
diff --git a/dev/tests/settings_local.py b/dev/tests/settings_local.py
index e1ffd60edb8..234e49aed2a 100644
--- a/dev/tests/settings_local.py
+++ b/dev/tests/settings_local.py
@@ -17,6 +17,7 @@
}
IDSUBMIT_IDNITS_BINARY = "/usr/local/bin/idnits"
+IDSUBMIT_IDNITS3_BINARY = "/usr/local/bin/idnits3"
IDSUBMIT_REPOSITORY_PATH = "/assets/ietfdata/doc/draft/repository"
IDSUBMIT_STAGING_PATH = "/assets/www6s/staging/"
diff --git a/docker/app.Dockerfile b/docker/app.Dockerfile
index dd4cf72ffd9..594b9d0e692 100644
--- a/docker/app.Dockerfile
+++ b/docker/app.Dockerfile
@@ -47,6 +47,14 @@ RUN groupmod --gid $USER_GID $USERNAME \
&& chown -R $USER_UID:$USER_GID /home/$USERNAME \
|| exit 0
+# Install idnits3. The version is pinned here and in the other container
+# definitions - grep for @ietf-tools/idnits to bump them together.
+# It goes in its own npm prefix so that it can never disturb the idnits2 script
+# at /usr/local/bin/idnits that comes from the base image.
+RUN npm install -g --prefix /usr/local/idnits3 @ietf-tools/idnits@3.1.0 && \
+ ln -sf /usr/local/idnits3/bin/idnits /usr/local/bin/idnits3 && \
+ idnits3 --version
+
# Switch to local dev user
USER dev:dev
diff --git a/docker/celery.Dockerfile b/docker/celery.Dockerfile
index e93ca3cf77c..a69ad8b5682 100644
--- a/docker/celery.Dockerfile
+++ b/docker/celery.Dockerfile
@@ -41,6 +41,14 @@ RUN groupmod --gid $USER_GID $USERNAME \
&& chown -R $USER_UID:$USER_GID /home/$USERNAME \
|| exit 0
+# Install idnits3. The version is pinned here and in the other container
+# definitions - grep for @ietf-tools/idnits to bump them together.
+# It goes in its own npm prefix so that it can never disturb the idnits2 script
+# at /usr/local/bin/idnits that comes from the base image.
+RUN npm install -g --prefix /usr/local/idnits3 @ietf-tools/idnits@3.1.0 && \
+ ln -sf /usr/local/idnits3/bin/idnits /usr/local/bin/idnits3 && \
+ idnits3 --version
+
# Switch to local dev user
USER dev:dev
diff --git a/docker/configs/settings_local.py b/docker/configs/settings_local.py
index 227da0a0ace..da8f23e35ea 100644
--- a/docker/configs/settings_local.py
+++ b/docker/configs/settings_local.py
@@ -24,6 +24,7 @@
}
IDSUBMIT_IDNITS_BINARY = "/usr/local/bin/idnits"
+IDSUBMIT_IDNITS3_BINARY = "/usr/local/bin/idnits3"
IDSUBMIT_STAGING_PATH = "/assets/www6s/staging/"
AGENDA_PATH = '/assets/www6s/proceedings/'
diff --git a/ietf/checks.py b/ietf/checks.py
index 3853e49f04e..099d2e88d92 100644
--- a/ietf/checks.py
+++ b/ietf/checks.py
@@ -54,6 +54,9 @@ def check_id_submission_files(app_configs, **kwargs):
return []
#
errors = []
+ hint = ("Please either update the local settings to point at the correct\n"
+ "\tfile, or if the setting is correct, make sure the file is in place and\n"
+ "\thas the right permissions.\n")
for s in ("IDSUBMIT_IDNITS_BINARY", ):
p = getattr(settings, s)
if not os.path.exists(p):
@@ -61,11 +64,22 @@ def check_id_submission_files(app_configs, **kwargs):
"A file used by the I-D submission tool does not exist\n"
"at the path given in the settings file. The setting is:\n"
" %s = %s" % (s, p),
- hint = ("Please either update the local settings to point at the correct\n"
- "\tfile, or if the setting is correct, make sure the file is in place and\n"
- "\thas the right permissions.\n"),
+ hint = hint,
id = "datatracker.E0007",
))
+ # The idnits3 check is advisory - a submission is not blocked by it, and is
+ # not blocked by its absence either, so only warn if it is not installed.
+ for s in ("IDSUBMIT_IDNITS3_BINARY", ):
+ p = getattr(settings, s)
+ if not os.path.exists(p):
+ errors.append(checks.Warning(
+ "A file used by the I-D submission tool does not exist\n"
+ "at the path given in the settings file. The advisory checks it\n"
+ "provides will be skipped. The setting is:\n"
+ " %s = %s" % (s, p),
+ hint = hint,
+ id = "datatracker.W0007",
+ ))
return errors
diff --git a/ietf/settings.py b/ietf/settings.py
index 3e26e22e840..24a2bf66216 100644
--- a/ietf/settings.py
+++ b/ietf/settings.py
@@ -954,6 +954,11 @@ def skip_unreadable_post(record):
IDSUBMIT_STAGING_PATH = '/a/www/www6s/staging/'
IDSUBMIT_STAGING_URL = '//www.ietf.org/staging/'
IDSUBMIT_IDNITS_BINARY = '/a/www/ietf-datatracker/scripts/idnits'
+IDSUBMIT_IDNITS3_BINARY = '/usr/local/bin/idnits3'
+# Set True to skip the idnits3 checks that need to fetch remote documents
+IDSUBMIT_IDNITS3_OFFLINE = False
+# Seconds to allow an idnits3 run before giving up on it
+IDSUBMIT_IDNITS3_TIMEOUT = 300
SUBMIT_PYANG_COMMAND = 'pyang --verbose --ietf -p {libs} {model}'
SUBMIT_YANGLINT_COMMAND = 'yanglint --verbose -p {tmplib} -p {rfclib} -p {draftlib} -p {ianalib} -p {cataloglib} {model} -i'
@@ -967,6 +972,7 @@ def skip_unreadable_post(record):
IDSUBMIT_CHECKER_CLASSES = (
"ietf.submit.checkers.DraftIdnitsChecker",
+ "ietf.submit.checkers.DraftIdnits3Checker",
"ietf.submit.checkers.DraftYangChecker",
# "ietf.submit.checkers.DraftYangvalidatorChecker",
)
diff --git a/ietf/submit/checkers.py b/ietf/submit/checkers.py
index e02b6865767..4c1bc24e93b 100644
--- a/ietf/submit/checkers.py
+++ b/ietf/submit/checkers.py
@@ -3,10 +3,12 @@
import io
+import json
import os
from pathlib import Path
import re
import shutil
+import subprocess
import sys
import tempfile
@@ -311,3 +313,169 @@ def check_file_txt(self, path):
info['items'] = items
info['code']['yang'] = model_list
return passed, message, errors, warnings, info
+
+
+class DraftIdnits3Checker(object):
+ """
+ Draft checker class for idnits3, run in "submission" mode.
+
+ idnits3 understands both text and XML Internet-Drafts, so both
+ check_file_xml() and check_file_txt() are defined; the XML is preferred
+ when the submission includes it, since that is the form the author wrote.
+
+ This checker is advisory: it never fails a submission, even when idnits3
+ reports errors that would block it once idnits3 becomes a required check.
+ It records whether it would have blocked in the check's `items` so that the
+ submitter can be warned. A run that could not be completed at all (missing
+ or broken binary, unparsable output) returns None for `passed`, which marks
+ the check as "did not apply" and hides it from the submitter.
+ """
+
+ name = "idnits3 check"
+
+ symbol = ""
+
+ _severities = (
+ ("ValidationError", "Errors"),
+ ("ValidationWarning", "Warnings"),
+ ("ValidationComment", "Comments"),
+ )
+
+ def __init__(self, options=None):
+ if options is None:
+ # --mode submission limits the checks to those relevant when a
+ # draft is submitted; --output json gives us reliable per-severity
+ # counts and structured nits to render ourselves.
+ options = ["--mode", "submission", "--output", "json", "--no-color"]
+ if settings.IDSUBMIT_IDNITS3_OFFLINE:
+ options.append("--offline")
+ assert isinstance(options, list)
+ self.options = options
+
+ def _version(self):
+ try:
+ result = subprocess.run(
+ [settings.IDSUBMIT_IDNITS3_BINARY, "--version"],
+ capture_output=True,
+ timeout=settings.IDSUBMIT_IDNITS3_TIMEOUT,
+ )
+ except (OSError, subprocess.SubprocessError):
+ return "unknown version"
+ if result.returncode != 0:
+ return "unknown version"
+ return result.stdout.decode("utf-8", errors="replace").strip()
+
+ def _render(self, nits, counts):
+ """Render the idnits3 nits as the text shown to the submitter"""
+ lines = [
+ "idnits %s (submission mode): %d error%s, %d warning%s, %d comment%s"
+ % (
+ self._version(),
+ counts["error"], "" if counts["error"] == 1 else "s",
+ counts["warning"], "" if counts["warning"] == 1 else "s",
+ counts["comment"], "" if counts["comment"] == 1 else "s",
+ ),
+ "",
+ "These results do not affect this submission. Errors reported here are",
+ "expected to prevent submission once idnits3 becomes a required check.",
+ "",
+ ]
+ if not nits:
+ lines.append("No nits found.")
+ return "\n".join(lines) + "\n"
+ index = 0
+ for severity, heading in self._severities:
+ of_severity = [n for n in nits if n.get("severity") == severity]
+ if not of_severity:
+ continue
+ lines.append("%s:" % heading)
+ lines.append("")
+ for nit in of_severity:
+ index += 1
+ indent = " " * 6
+ lines.append("%4d. %s" % (index, nit.get("code", "UNKNOWN")))
+ lines.append("%s%s" % (indent, nit.get("desc", "")))
+ if nit.get("text"):
+ lines.append("%sText: %s" % (indent, nit["text"]))
+ if nit.get("path"):
+ lines.append("%sPath: %s" % (indent, nit["path"]))
+ if nit.get("line"):
+ lines.append(
+ "%sAt: %s"
+ % (
+ indent,
+ ", ".join(
+ "line %s column %s" % (loc.get("line"), loc.get("pos"))
+ for loc in nit["line"]
+ ),
+ )
+ )
+ if nit.get("ref"):
+ lines.append("%sSee %s" % (indent, nit["ref"]))
+ lines.append("")
+ return "\n".join(lines) + "\n"
+
+ def _check_file(self, path):
+ info = {
+ "checker": self.name,
+ "items": [],
+ "code": {},
+ "advisory": True,
+ "would_block_in_future": False,
+ }
+ cmd = [settings.IDSUBMIT_IDNITS3_BINARY] + self.options + [str(path)]
+ try:
+ result = subprocess.run(
+ cmd, capture_output=True, timeout=settings.IDSUBMIT_IDNITS3_TIMEOUT
+ )
+ except (OSError, subprocess.SubprocessError) as err:
+ message = "idnits3 error: %s:\n %s" % (" ".join(cmd), err)
+ log(message)
+ return None, message, 0, 0, info
+ if result.returncode != 0:
+ message = "idnits3 error: %s:\n Error %s: %s" % (
+ " ".join(cmd),
+ result.returncode,
+ result.stderr.decode("utf-8", errors="replace"),
+ )
+ log(message)
+ return None, message, 0, 0, info
+ try:
+ output = json.loads(result.stdout.decode("utf-8", errors="replace"))
+ counts = {
+ severity: int(output["nitsBySeverity"].get(severity, 0))
+ for severity in ("error", "warning", "comment")
+ }
+ nits = output.get("nits", [])
+ if not isinstance(nits, list) or not all(
+ isinstance(nit, dict) for nit in nits
+ ):
+ raise ValueError("'nits' is not a list of nits")
+ except (AttributeError, KeyError, TypeError, ValueError) as err:
+ message = "idnits3 error: %s:\n Could not parse the idnits3 output: %s" % (
+ " ".join(cmd),
+ err,
+ )
+ log(message)
+ return None, message, 0, 0, info
+
+ info["items"] = [
+ (
+ nit["line"][0]["line"] if nit.get("line") else None,
+ None,
+ "%s: %s" % (nit.get("code", "UNKNOWN"), nit.get("desc", "")),
+ )
+ for nit in nits
+ ]
+ info["would_block_in_future"] = counts["error"] > 0
+ # Comments are neither errors nor warnings, but reporting them as
+ # warnings is the only way to get them in front of the submitter.
+ warnings = counts["warning"] + counts["comment"]
+ # Always passes -- idnits3 does not block submission yet.
+ return True, self._render(nits, counts), counts["error"], warnings, info
+
+ def check_file_txt(self, path):
+ return self._check_file(path)
+
+ def check_file_xml(self, path):
+ return self._check_file(path)
diff --git a/ietf/submit/models.py b/ietf/submit/models.py
index 576ba3e1143..355b92c9fd1 100644
--- a/ietf/submit/models.py
+++ b/ietf/submit/models.py
@@ -130,21 +130,32 @@ def closed_wg_drafts_replaced(self):
class SubmissionCheck(models.Model):
time = models.DateTimeField(default=timezone.now)
- submission = ForeignKey(Submission, related_name='checks')
+ submission = ForeignKey(Submission, related_name="checks")
checker = models.CharField(max_length=256, blank=True)
passed = models.BooleanField(null=True, default=False)
message = models.TextField(null=True, blank=True)
errors = models.IntegerField(null=True, blank=True, default=None)
warnings = models.IntegerField(null=True, blank=True, default=None)
items = models.JSONField(null=True, blank=True, default=dict)
- symbol = models.CharField(max_length=64, default='')
- #
+ symbol = models.CharField(max_length=64, default="")
+
def __str__(self):
- return "%s submission check: %s: %s" % (self.checker, 'Passed' if self.passed else 'Failed', self.message[:48]+'...')
- def has_warnings(self):
- return self.warnings != '[]'
- def has_errors(self):
- return self.errors != '[]'
+ return (
+ f"{self.checker} submission check: "
+ f"{'Passed' if self.passed else 'Failed'}: {(self.message or '')[:48]}..."
+ )
+
+ @property
+ def is_advisory(self):
+ """Is this a check whose result cannot prevent a submission?"""
+ return bool(isinstance(self.items, dict) and self.items.get("advisory"))
+
+ @property
+ def would_block_in_future(self):
+ """Would this advisory check have blocked the submission if it were required?"""
+ return bool(
+ isinstance(self.items, dict) and self.items.get("would_block_in_future")
+ )
class SubmissionEvent(models.Model):
submission = ForeignKey(Submission)
diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py
index abe23c1a643..28d46d0b2a4 100644
--- a/ietf/submit/tests.py
+++ b/ietf/submit/tests.py
@@ -5,10 +5,13 @@
import datetime
import email
import io
+import json
from unittest import mock
import os
import re
+import shutil
import sys
+import tempfile
from io import StringIO
from pyquery import PyQuery
@@ -41,6 +44,7 @@
from ietf.name.models import DraftSubmissionStateName, FormalLanguageName
from ietf.person.models import Person
from ietf.person.factories import UserFactory, PersonFactory, EmailFactory
+from ietf.submit.checkers import DraftIdnits3Checker
from ietf.submit.factories import SubmissionFactory, SubmissionExtResourceFactory
from ietf.submit.forms import SubmissionBaseUploadForm, SubmissionAutoUploadForm
from ietf.submit.models import Submission, Preapproval, SubmissionExtResource
@@ -3460,6 +3464,240 @@ def test_submission_checks(self):
status_code=200,
)
+ def test_advisory_submission_checks(self):
+ """An advisory check reports its result but does not block the submission"""
+ submission = SubmissionFactory(state_id="uploaded")
+ url = urlreverse(
+ "ietf.submit.views.submission_status",
+ kwargs={"submission_id": submission.pk},
+ )
+ check = submission.checks.create(
+ checker="idnits3 check",
+ passed=True,
+ message="idnits3 message",
+ errors=2,
+ warnings=1,
+ items={"advisory": True, "would_block_in_future": True},
+ )
+ r = self.client.get(url)
+ # The submission still passes - the advisory check does not block it ...
+ self.assertContains(
+ r, "Your Internet-Draft has been verified to pass the submission checks."
+ )
+ # ... but the submitter is warned that it would in the future
+ self.assertContains(r, "The idnits3 check returned 2 errors")
+ self.assertContains(r, "and 1 warning.")
+ self.assertContains(r, "would then be rejected")
+ self.assertContains(r, "idnits3 message")
+
+ # Warnings only - nothing that would block later
+ check.errors = 0
+ check.items = {"advisory": True, "would_block_in_future": False}
+ check.save()
+ r = self.client.get(url)
+ self.assertNotContains(r, "would then be rejected")
+ self.assertContains(r, "The idnits3 check returned 1 warning.")
+ self.assertContains(r, "None of them would stop this submission.")
+
+ # Nothing at all to report
+ check.warnings = 0
+ check.save()
+ r = self.client.get(url)
+ self.assertNotContains(r, "would then be rejected")
+ self.assertNotContains(r, "None of them would stop this submission.")
+
+
+class Idnits3CheckerTests(TestCase):
+ """Tests of DraftIdnits3Checker
+
+ Most of these run the checker against a stand-in for the idnits3 binary so that
+ they neither depend on idnits3 being installed nor on network access.
+ """
+
+ def setUp(self):
+ super().setUp()
+ self.tempdir = Path(tempfile.mkdtemp())
+ self.addCleanup(shutil.rmtree, self.tempdir)
+ self.draft_path = self.tempdir / "draft-somebody-test-00.txt"
+ self.draft_path.write_text("Not really an Internet-Draft\n")
+ self.args_path = self.tempdir / "args"
+
+ def fake_idnits3(self, stdout="", returncode=0):
+ """Write a stand-in for the idnits3 binary and return its path
+
+ The stand-in records the arguments it was called with in self.args_path.
+ """
+ path = self.tempdir / "idnits3"
+ path.write_text(
+ "#!/bin/sh\n"
+ 'if [ "$1" = "--version" ]; then echo "3.1.0"; exit 0; fi\n'
+ f'echo "$@" > {self.args_path}\n'
+ "cat <<'IDNITS3_EOF'\n"
+ f"{stdout}\n"
+ "IDNITS3_EOF\n"
+ f"exit {returncode}\n"
+ )
+ path.chmod(0o755)
+ return str(path)
+
+ @staticmethod
+ def idnits3_output(nits):
+ counts = {"error": 0, "warning": 0, "comment": 0}
+ for nit in nits:
+ counts[nit["severity"].replace("Validation", "").lower()] += 1
+ return json.dumps(
+ {
+ "result": "fail" if nits else "pass",
+ "file": {"path": "draft-somebody-test-00.txt", "size": 29},
+ "nitsBySeverity": counts,
+ "nits": nits,
+ }
+ )
+
+ def test_reports_nits_without_blocking(self):
+ output = self.idnits3_output(
+ [
+ {
+ "severity": "ValidationError",
+ "code": "MISSING_ABSTRACT_SECTION",
+ "desc": "The abstract section is missing.",
+ "ref": "https://authors.ietf.org/required-content#abstract",
+ "path": "rfc.front.abstract",
+ },
+ {
+ "severity": "ValidationWarning",
+ "code": "LINE_TOO_LONG",
+ "desc": "The document contains 1 over-long line.",
+ "line": [{"line": 7, "pos": 80}],
+ },
+ {
+ "severity": "ValidationComment",
+ "code": "SOME_COMMENT",
+ "desc": "Just a comment.",
+ },
+ ]
+ )
+ with override_settings(IDSUBMIT_IDNITS3_BINARY=self.fake_idnits3(output)):
+ passed, message, errors, warnings, info = DraftIdnits3Checker().check_file_txt(
+ self.draft_path
+ )
+ # The check must not fail the submission, even though idnits3 found errors
+ self.assertTrue(passed)
+ self.assertEqual(errors, 1)
+ self.assertEqual(warnings, 2) # warnings and comments are both reported as warnings
+ self.assertTrue(info["advisory"])
+ self.assertTrue(info["would_block_in_future"])
+ self.assertEqual(len(info["items"]), 3)
+ self.assertIn("MISSING_ABSTRACT_SECTION", message)
+ self.assertIn("The abstract section is missing.", message)
+ self.assertIn("rfc.front.abstract", message)
+ self.assertIn("https://authors.ietf.org/required-content#abstract", message)
+ self.assertIn("line 7 column 80", message)
+ self.assertIn("SOME_COMMENT", message)
+ self.assertIn("3.1.0", message)
+
+ def test_would_not_block_without_errors(self):
+ output = self.idnits3_output(
+ [
+ {
+ "severity": "ValidationWarning",
+ "code": "LINE_TOO_LONG",
+ "desc": "The document contains 1 over-long line.",
+ }
+ ]
+ )
+ with override_settings(IDSUBMIT_IDNITS3_BINARY=self.fake_idnits3(output)):
+ passed, message, errors, warnings, info = DraftIdnits3Checker().check_file_txt(
+ self.draft_path
+ )
+ self.assertTrue(passed)
+ self.assertEqual(errors, 0)
+ self.assertEqual(warnings, 1)
+ self.assertFalse(info["would_block_in_future"])
+
+ def test_clean_document(self):
+ with override_settings(
+ IDSUBMIT_IDNITS3_BINARY=self.fake_idnits3(self.idnits3_output([]))
+ ):
+ passed, message, errors, warnings, info = DraftIdnits3Checker().check_file_txt(
+ self.draft_path
+ )
+ self.assertTrue(passed)
+ self.assertEqual((errors, warnings), (0, 0))
+ self.assertFalse(info["would_block_in_future"])
+ self.assertIn("No nits found.", message)
+
+ def test_runs_in_submission_mode(self):
+ with override_settings(
+ IDSUBMIT_IDNITS3_BINARY=self.fake_idnits3(self.idnits3_output([]))
+ ):
+ DraftIdnits3Checker().check_file_xml(self.draft_path)
+ args = self.args_path.read_text().split()
+ self.assertEqual(args[:2], ["--mode", "submission"])
+ self.assertIn("--output", args)
+ self.assertIn("json", args)
+ self.assertNotIn("--offline", args)
+ self.assertEqual(args[-1], str(self.draft_path))
+
+ def test_offline_setting(self):
+ with override_settings(IDSUBMIT_IDNITS3_OFFLINE=True):
+ self.assertIn("--offline", DraftIdnits3Checker().options)
+ with override_settings(IDSUBMIT_IDNITS3_OFFLINE=False):
+ self.assertNotIn("--offline", DraftIdnits3Checker().options)
+
+ def test_missing_binary_does_not_apply(self):
+ with override_settings(
+ IDSUBMIT_IDNITS3_BINARY=str(self.tempdir / "there-is-no-idnits3")
+ ):
+ passed, message, errors, warnings, info = DraftIdnits3Checker().check_file_txt(
+ self.draft_path
+ )
+ self.assertIsNone(passed) # "did not apply", so the submitter is not shown it
+ self.assertEqual((errors, warnings), (0, 0))
+ self.assertFalse(info["would_block_in_future"])
+ self.assertIn("idnits3 error", message)
+
+ def test_failed_run_does_not_apply(self):
+ with override_settings(
+ IDSUBMIT_IDNITS3_BINARY=self.fake_idnits3("boom", returncode=1)
+ ):
+ passed, message, __, __, info = DraftIdnits3Checker().check_file_txt(
+ self.draft_path
+ )
+ self.assertIsNone(passed)
+ self.assertFalse(info["would_block_in_future"])
+ self.assertIn("idnits3 error", message)
+
+ def test_unparsable_output_does_not_apply(self):
+ with override_settings(
+ IDSUBMIT_IDNITS3_BINARY=self.fake_idnits3("this is not json")
+ ):
+ passed, message, __, __, info = DraftIdnits3Checker().check_file_txt(
+ self.draft_path
+ )
+ self.assertIsNone(passed)
+ self.assertFalse(info["would_block_in_future"])
+ self.assertIn("Could not parse the idnits3 output", message)
+
+ @override_settings(IDSUBMIT_IDNITS3_OFFLINE=True)
+ def test_real_idnits3(self):
+ """The real idnits3 produces output this checker can make sense of"""
+ if not os.path.exists(settings.IDSUBMIT_IDNITS3_BINARY):
+ # idnits3 is installed into the datatracker and celery images, and
+ # into the CI test container by dev/tests/prepare.sh - but it is not
+ # in the base image, so a bare base container will not have it
+ self.skipTest(f"idnits3 is not installed at {settings.IDSUBMIT_IDNITS3_BINARY}")
+ name = "draft-somebody-test-idnits3-00"
+ text, __ = submission_file_contents(name, None, "test_submission.txt")
+ path = self.tempdir / f"{name}.txt"
+ path.write_text(text)
+ passed, message, errors, warnings, info = DraftIdnits3Checker().check_file_txt(path)
+ self.assertTrue(passed) # idnits3 never blocks a submission
+ self.assertTrue(info["advisory"])
+ self.assertEqual(info["would_block_in_future"], errors > 0)
+ self.assertEqual(len(info["items"]), errors + warnings)
+ self.assertIn("submission mode", message)
+
class YangCheckerTests(TestCase):
@mock.patch("ietf.submit.utils.apply_yang_checker_to_draft")
diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py
index b331d71631f..0ddd3a09efc 100644
--- a/ietf/submit/utils.py
+++ b/ietf/submit/utils.py
@@ -829,10 +829,16 @@ def apply_check(submission, checker, method, fn):
message=message, errors=errors, warnings=warnings, items=info,
symbol=checker.symbol)
check.save()
- # ordered list of methods to try
+ # ordered list of methods to try - skip formats that were not submitted or
+ # generated, so that (e.g.) a checker that prefers XML still runs on the
+ # text of a submission that has no XML
for method in ("check_fragment_xml", "check_file_xml", "check_fragment_txt", "check_file_txt", ):
ext = method[-3:]
- if hasattr(checker, method) and ext in file_name:
+ if (
+ hasattr(checker, method)
+ and ext in file_name
+ and os.path.exists(file_name[ext])
+ ):
apply_check(submission, checker, method, file_name[ext])
break
diff --git a/ietf/templates/submit/submission_status.html b/ietf/templates/submit/submission_status.html
index cdc5dd4007e..8ecbe80e23f 100644
--- a/ietf/templates/submit/submission_status.html
+++ b/ietf/templates/submit/submission_status.html
@@ -59,7 +59,23 @@
Submission checks
{% endif %}
{% for check in submission.latest_checks %}
- {% if check.errors %}
+ {% if check.is_advisory %}
+ {% if check.would_block_in_future %}
+
+ The {{ check.checker }} returned {{ check.errors }} error{{ check.errors|pluralize }}
+ and {{ check.warnings }} warning{{ check.warnings|pluralize }}.
+ The {{ check.checker }} does not affect this submission, so those errors have
+ not stopped it. It is expected to become a required submission check,
+ however, and this Internet-Draft would then be rejected. Click the button
+ below to see details, and please fix those before you submit again.
+
+ {% elif check.warnings %}
+
+ The {{ check.checker }} returned {{ check.warnings }} warning{{ check.warnings|pluralize }}.
+ None of them would stop this submission.
+
+ {% endif %}
+ {% elif check.errors %}
The {{ check.checker }} returned {{ check.errors }} error{{ check.errors|pluralize }}
and {{ check.warnings }} warning{{ check.warnings|pluralize }}; click the button
@@ -72,7 +88,7 @@
Submission checks
{% endif %}
{% endfor %}
{% for check in submission.latest_checks %}
-