tests should not use "real" filesystem data

[ietf-svn-bot]

owner:jennifer@painless-security.com resolution_fixed type_defect | by rjsparks@nostrum.com

---

There are tests that currently use what they find in the configured INTERNET_ALL_DRAFTS_ARCHIVE_DIR without overriding the setting and providing test data. This adds a point of instability, and makes the tests unnecessarily slow when it tries to process every file it sees in that directory (which will typically be a replica of the entire id-archive on a developer's machine).

---

Issue migrated from trac:3414 at 2022-03-04 09:14:52 +0000

[ietf-svn-bot]

@rjsparks@nostrum.com changed priority from medium to major

[ietf-svn-bot]

@rjsparks@nostrum.com changed status from new to accepted

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

ietf.doc.tests.DocDraftTestCase.test_document_draft will fail unless there is an rfc with number matching what the factory generates lying around at RFC_PATH.

This is wrong, RFC_PATH should be overridden for this (and all) tests, and test data placed in the overridden directory as necessary.

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

In Lars' newest container, this file needs to exist for the tests to pass (until the test is updated to not do the wrong thing):
$ touch test/rfc/rfc1007.txt

[ietf-svn-bot]

@jennifer@painless-security.com changed status from accepted to assigned

[ietf-svn-bot]

@jennifer@painless-security.com set owner to jennifer@painless-security.com

[ietf-svn-bot]

@jennifer@painless-security.com commented

---

As of 7.39.0, two tests seem to be failing when I set RFC_PATH to something that does not exist: test_document_draft and test_document_stats.

[ietf-svn-bot]

@jennifer@painless-security.com commented

---

Should we override the settings for all tests as a precaution or fix it on the affected tests? The RFC_PATH does not seem to be widely needed, not sure about the INTERNET_ALL_DRAFTS_ARCHIVE_DIR yet.

I notice that if I change all of IDSUBMIT_IDNITS_BINARY, IDSUBMIT_REPOSITORY_PATH, IDSUBMIT_STAGING_PATH, INTERNET_DRAFT_ARCHIVE_DIR, INTERNET_ALL_DRAFTS_ARCHIVE_DIR, and RFC_PATH,
the system check test complains and the other tests are skipped. Do we want to address any of those while in the area?

SystemCheckError: System check identified some issues:

CRITICALS:
?: (datatracker.E0006) A directory used by the ID submission tool does not
exist at the path given in the settings file.  The setting is:
    IDSUBMIT_REPOSITORY_PATH = test/id-huhh/
	HINT: Please either update the local settings to point at the correct
	directory, or if the setting is correct, create the indicated directory.

?: (datatracker.E0006) A directory used by the ID submission tool does not
exist at the path given in the settings file.  The setting is:
    IDSUBMIT_STAGING_PATH = test/staging-whaa/
	HINT: Please either update the local settings to point at the correct
	directory, or if the setting is correct, create the indicated directory.

?: (datatracker.E0006) A directory used by the ID submission tool does not
exist at the path given in the settings file.  The setting is:
    INTERNET_DRAFT_ARCHIVE_DIR = test/archive-uh-uh/
	HINT: Please either update the local settings to point at the correct
	directory, or if the setting is correct, create the indicated directory.

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

Take the pragmatic approach - if it's not a huge effort to identify and fix the rest of the places where the tests are looking at real data, lets be comprehensive.

Otherwise, lets fix where it hurts us now and remember to watch for it hurting in the future.

The pressure for identifying and fixing all of them will turn up a bit when we build the next CI system - it would be nice to have that run on a container that doesn't need actual production filesystem data (or a dump of the production database).

[ietf-svn-bot]

@jennifer@painless-security.com commented

---

Makes sense, thanks

[ietf-svn-bot]

@jennifer@painless-security.com commented

---

It seems to be straightforward to set up the ietf.utils.test_utils.TestCase subclass to create temp directories and override the settings for all the tests. This should prevent accidental reliance on the filesystem or side effects of other tests.

The main wrinkle is that it means TestCase classes with their own setUp() and tearDown() methods need to be updated to call the superclass methods. That should be simple enough (and good discipline anyway).

[ietf-svn-bot]

@jennifer@painless-security.com changed status from assigned to closed

[ietf-svn-bot]

@jennifer@painless-security.com set resolution to fixed

[ietf-svn-bot]

@jennifer@painless-security.com commented

---

Fixed in 81d9234:

Use temporary directories instead of "real" filesystem for tests. Fixes #3414. Commit ready for merge.