Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Prevent test suite artifact creation in work directory #6438

Merged
merged 25 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
bd4cb9a
chore: Prevent test suite artifact creation in work directory
larseggert Oct 5, 2023
adbdee1
Try and fix CI
larseggert Oct 5, 2023
02f1c5f
Merge branch 'main' into chore-test-use-tmp
larseggert Oct 11, 2023
2c3ebaa
Change IDSUBMIT_REPOSITORY_PATH
larseggert Oct 11, 2023
a291735
Merge branch 'main' into chore-test-use-tmp
larseggert Oct 12, 2023
954a27a
Make the dir
larseggert Oct 12, 2023
a18c1f8
Merge branch 'main' into chore-test-use-tmp
larseggert Oct 23, 2023
5afd314
Merge branch 'main' into chore-test-use-tmp
larseggert Oct 24, 2023
53dadf1
Merge branch 'main' into chore-test-use-tmp
larseggert Oct 25, 2023
68dd89a
Merge branch 'main' into chore-test-use-tmp
larseggert Oct 28, 2023
b5cfb38
Merge branch 'main' into chore-test-use-tmp
larseggert Nov 4, 2023
b6e933b
Merge branch 'main' into chore-test-use-tmp
larseggert Nov 9, 2023
3a74f9a
Merge branch 'main' into chore-test-use-tmp
larseggert Nov 10, 2023
18441d2
Merge branch 'main' into chore-test-use-tmp
larseggert Dec 8, 2023
c258a59
Merge branch 'main' into chore-test-use-tmp
larseggert Dec 12, 2023
5ae532c
Merge branch 'main' into chore-test-use-tmp
larseggert Jan 2, 2024
43808a1
Merge branch 'main' into chore-test-use-tmp
jennifer-richards Sep 9, 2024
9c4712f
Merge branch 'refs/heads/main' into fork/larseggert/chore-test-use-tmp
jennifer-richards Sep 16, 2024
e9bc0ae
test: clean up media/nomcom directories
jennifer-richards Sep 16, 2024
1f329bf
test: de-dup settings_temp_path_overrides
jennifer-richards Sep 16, 2024
0c4b97a
chore: remove debug
jennifer-richards Sep 16, 2024
85543c6
Merge branch 'main' into fork/larseggert/chore-test-use-tmp
jennifer-richards Sep 16, 2024
64a27f4
refactor: avoid premature import of test_utils
jennifer-richards Sep 17, 2024
b0648c4
Merge branch 'main' into chore-test-use-tmp
jennifer-richards Sep 17, 2024
6261c7b
refactor: drop useless lambda wrapper
jennifer-richards Sep 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dev/tests/settings_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
}

IDSUBMIT_IDNITS_BINARY = "/usr/local/bin/idnits"
IDSUBMIT_REPOSITORY_PATH = "test/id/"
IDSUBMIT_STAGING_PATH = "test/staging/"
IDSUBMIT_REPOSITORY_PATH = "/assets/ietfdata/doc/draft/repository"
IDSUBMIT_STAGING_PATH = "/assets/www6s/staging/"
larseggert marked this conversation as resolved.
Show resolved Hide resolved

AGENDA_PATH = '/assets/www6s/proceedings/'
MEETINGHOST_LOGO_PATH = AGENDA_PATH
Expand Down
4 changes: 2 additions & 2 deletions docker/configs/settings_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from ietf.settings_postgresqldb import DATABASES # pyflakes:ignore

IDSUBMIT_IDNITS_BINARY = "/usr/local/bin/idnits"
IDSUBMIT_STAGING_PATH = "test/staging/"
IDSUBMIT_STAGING_PATH = "/assets/www6s/staging/"

AGENDA_PATH = '/assets/www6s/proceedings/'
MEETINGHOST_LOGO_PATH = AGENDA_PATH
Expand Down Expand Up @@ -52,7 +52,7 @@
IDSUBMIT_REPOSITORY_PATH = INTERNET_DRAFT_PATH

NOMCOM_PUBLIC_KEYS_DIR = 'data/nomcom_keys/public_keys/'
SLIDE_STAGING_PATH = 'test/staging/'
SLIDE_STAGING_PATH = '/assets/www6s/staging/'

DE_GFM_BINARY = '/usr/local/bin/de-gfm'

Expand Down
8 changes: 1 addition & 7 deletions docker/scripts/app-create-dirs.sh
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
#!/bin/bash

for sub in \
test/id \
test/staging \
test/archive \
test/rfc \
test/media \
test/wiki/ietf \
data/nomcom_keys/public_keys \
/assets/archive/id \
/assets/ietf-ftp \
/assets/ietf-ftp/bofreq \
Expand All @@ -25,6 +18,7 @@ for sub in \
/assets/ietfdata/derived \
/assets/ietfdata/derived/bibxml \
/assets/ietfdata/derived/bibxml/bibxml-ids \
/assets/ietfdata/doc/draft/repository \
/assets/www6s \
/assets/www6s/staging \
/assets/www6s/wg-descriptions \
Expand Down
11 changes: 3 additions & 8 deletions ietf/meeting/tests_js.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import time
import datetime
import shutil
import os
import tempfile
import re

from django.utils import timezone
Expand Down Expand Up @@ -939,13 +939,8 @@ def tearDown(self):
def tempdir(self, label):
# Borrowed from test_utils.TestCase
slug = slugify(self.__class__.__name__.replace('.','-'))
dirname = "tmp-{label}-{slug}-dir".format(**locals())
if 'VIRTUAL_ENV' in os.environ:
dirname = os.path.join(os.environ['VIRTUAL_ENV'], dirname)
path = os.path.abspath(dirname)
if not os.path.exists(path):
os.mkdir(path)
return path
suffix = "-{label}-{slug}-dir".format(**locals())
return tempfile.mkdtemp(suffix=suffix)

def displayed_interims(self, groups=None):
sessions = add_event_info_to_session_qs(
Expand Down
7 changes: 3 additions & 4 deletions ietf/person/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import faker.config
import os
import random
import shutil
from PIL import Image

from unidecode import unidecode
from unicodedata import normalize
Expand Down Expand Up @@ -103,10 +103,9 @@ def default_photo(obj, create, extracted, **kwargs): # pylint: disable=no-self-a
media_name = "%s/%s.jpg" % (settings.PHOTOS_DIRNAME, photo_name)
obj.photo = media_name
obj.photo_thumb = media_name
photosrc = os.path.join(settings.TEST_DATA_DIR, "profile-default.jpg")
photodst = os.path.join(settings.PHOTOS_DIR, photo_name + '.jpg')
if not os.path.exists(photodst):
shutil.copy(photosrc, photodst)
img = Image.new('RGB', (200, 200))
img.save(photodst)
Comment on lines +107 to +108
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remind me why we're not just making a copy of the profile-default.jpg again?
(I'm sensitive to the PIL import - it's large)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's just avoiding a file that (as far as I can tell) exists only for the tests.

I don't have an opinion one way or the other, but maybe there's a lighter weight library for generating an empty image for testing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's PyPNG - much smaller it seems, but requires less clearly intentioned code to generate a blank image and is somewhat slower on a per-image basis. The import of PIL on my system looks to be a few tens of ms - and I think we probably only hit that once per test run. (Though I haven't actually done a profile to see if the import is being exercised more than once; not sure how to do that)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect there's a valid, small, literal we can use as a PNG, but not going to slow this down over it.
I think we should, however, spend time scrubbing our full dependency set and seeing if we can make it smaller, and PIL is one of the things I'm wondering if we can scrub completely away.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use PIL for resizing images in Person.photo. If we do away with that (mis?)feature, then we can probably ditch even needing to fake an image.

Checked and PyPNG is a < 40kb python file. It can read and write PNGs, so if we do need to create one for some reason, it's a pretty minimal bit of baggage.

def delete_file(file):
os.unlink(file)
atexit.register(delete_file, photodst)
Expand Down
12 changes: 7 additions & 5 deletions ietf/settings_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
#

import os
import tempfile
from ietf.settings import * # pyflakes:ignore
from ietf.settings import TEST_CODE_COVERAGE_CHECKER, BASE_DIR, PHOTOS_DIRNAME
from ietf.settings import TEST_CODE_COVERAGE_CHECKER
import debug # pyflakes:ignore
debug.debug = True

Expand Down Expand Up @@ -48,11 +49,12 @@ def __getitem__(self, item):
if TEST_CODE_COVERAGE_CHECKER and not TEST_CODE_COVERAGE_CHECKER._started: # pyflakes:ignore
TEST_CODE_COVERAGE_CHECKER.start() # pyflakes:ignore

NOMCOM_PUBLIC_KEYS_DIR=os.path.abspath("tmp-nomcom-public-keys-dir")
NOMCOM_PUBLIC_KEYS_DIR = tempfile.mkdtemp(suffix="-nomcom-public-keys-dir")
jennifer-richards marked this conversation as resolved.
Show resolved Hide resolved

MEDIA_ROOT = os.path.join(os.path.dirname(BASE_DIR), 'test/media/') # pyflakes:ignore
MEDIA_URL = '/test/media/'
PHOTOS_DIR = MEDIA_ROOT + PHOTOS_DIRNAME # pyflakes:ignore
MEDIA_ROOT = tempfile.mkdtemp(suffix="-media")
jennifer-richards marked this conversation as resolved.
Show resolved Hide resolved
PHOTOS_DIRNAME = "photo"
PHOTOS_DIR = os.path.join(MEDIA_ROOT, PHOTOS_DIRNAME)
os.mkdir(PHOTOS_DIR)

# Undo any developer-dependent middleware when running the tests
MIDDLEWARE = [ c for c in MIDDLEWARE if not c in DEV_MIDDLEWARE ] # pyflakes:ignore
Expand Down
11 changes: 3 additions & 8 deletions ietf/utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.


import os
import tempfile
import re
import email
import html5lib
Expand Down Expand Up @@ -238,13 +238,8 @@ def normalize(x):

def tempdir(self, label):
slug = slugify(self.__class__.__name__.replace('.','-'))
dirname = "tmp-{label}-{slug}-dir".format(**locals())
if 'VIRTUAL_ENV' in os.environ:
dirname = os.path.join(os.environ['VIRTUAL_ENV'], dirname)
path = os.path.abspath(dirname)
if not os.path.exists(path):
os.mkdir(path)
return path
suffix = "-{label}-{slug}-dir".format(**locals())
return tempfile.mkdtemp(suffix=suffix)

def assertNoFormPostErrors(self, response, error_css_selector=".is-invalid"):
"""Try to fish out form errors, if none found at least check the
Expand Down
1 change: 0 additions & 1 deletion media/.gitignore

This file was deleted.

Binary file removed media/photo/nopictureavailable.jpg
Binary file not shown.
Binary file removed media/photo/profile-default.jpg
rjsparks marked this conversation as resolved.
Show resolved Hide resolved
Binary file not shown.
Binary file removed test/data/profile-default.jpg
Binary file not shown.
Loading
Loading