From 29fb109b6f5ab013d6c3a3239627e63717b5ede2 Mon Sep 17 00:00:00 2001
From: Bastiaan Welmers <bastiaan@welmers.net>
Date: Thu, 21 May 2020 18:05:08 +0200
Subject: [PATCH 1/7] Helper function to improve code

---
 tests/videodinges/factories.py | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tests/videodinges/factories.py b/tests/videodinges/factories.py
index a87cafb..93059d2 100644
--- a/tests/videodinges/factories.py
+++ b/tests/videodinges/factories.py
@@ -11,7 +11,7 @@ T = TypeVar('T', bound=django.db.models.Model)
 
 def create(model: Type[T], **kwargs) -> T:
 	if model is models.Video:
-		return models.Video.objects.create(**{**dict(title='Title', slug='slug', description='Description'), **kwargs})
+		return _create_with_defaults(models.Video, kwargs, title='Title', slug='slug', description='Description')
 
 	if model is models.Transcoding:
 		video = create(models.Video, title='Title', slug='slug', description='Description') \
@@ -25,12 +25,19 @@ def create(model: Type[T], **kwargs) -> T:
 			# only URL if no upload for they are multually exclusive
 			defaults['url'] = 'https://some_url'
 
-		return models.Transcoding.objects.create(**{**defaults, **kwargs})
+		return _create_with_defaults(models.Transcoding, kwargs, **defaults)
 
 	if model is models.Upload:
 		file = SimpleUploadedFile('some_file.txt', b'some contents') \
 			if 'file' not in kwargs else None
-		return models.Upload.objects.create(**{**dict(file=file), **kwargs})
+		return _create_with_defaults(models.Upload, kwargs, file=file)
 
 
-# TODO fix annoying dict notation to something more gentle.
+def _create_with_defaults(model: Type[T], kwargs: dict, **defaults) -> T:
+	"""
+		Return created django model instance.
+		:param model: django model to create
+		:param kwargs: keyword arguments to fill the model
+		:param defaults: default keyword arguments to use when not mentioned in kwargs
+	"""
+	return model.objects.create(**{**defaults, **kwargs})

From 8c4ec83993005c775c60b280698d1a8c37a1114b Mon Sep 17 00:00:00 2001
From: Bastiaan Welmers <bastiaan@welmers.net>
Date: Thu, 21 May 2020 18:36:21 +0200
Subject: [PATCH 2/7] Use lambda for dependencies

so they won't be created when dependency is already provided
---
 tests/videodinges/factories.py                       | 12 +++++++++---
 tests/videodinges/test_factories/test_transcoding.py | 11 +++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/tests/videodinges/factories.py b/tests/videodinges/factories.py
index 93059d2..458bb89 100644
--- a/tests/videodinges/factories.py
+++ b/tests/videodinges/factories.py
@@ -14,10 +14,8 @@ def create(model: Type[T], **kwargs) -> T:
 		return _create_with_defaults(models.Video, kwargs, title='Title', slug='slug', description='Description')
 
 	if model is models.Transcoding:
-		video = create(models.Video, title='Title', slug='slug', description='Description') \
-			if 'video' not in kwargs else None
 		defaults = dict(
-			video=video,
+			video=lambda: create(models.Video),
 			quality=models.qualities[0].name,
 			type=str(models.transcoding_types[0]),
 		)
@@ -36,8 +34,16 @@ def create(model: Type[T], **kwargs) -> T:
 def _create_with_defaults(model: Type[T], kwargs: dict, **defaults) -> T:
 	"""
 		Return created django model instance.
+		When providing lambda as default item, the result of the lambda will be taken.
+		The lambda will ONLY be executed when not provided in kwargs.
+
 		:param model: django model to create
 		:param kwargs: keyword arguments to fill the model
 		:param defaults: default keyword arguments to use when not mentioned in kwargs
 	"""
+
+	for k, v in defaults.items():
+		if callable(v) and not k in kwargs:
+			defaults[k] = v()
+
 	return model.objects.create(**{**defaults, **kwargs})
diff --git a/tests/videodinges/test_factories/test_transcoding.py b/tests/videodinges/test_factories/test_transcoding.py
index 0ddaca6..b1f691f 100644
--- a/tests/videodinges/test_factories/test_transcoding.py
+++ b/tests/videodinges/test_factories/test_transcoding.py
@@ -24,3 +24,14 @@ class TranscodingTestCase(TestCase):
 		self.assertEqual(transcoding.quality, '720p')
 		self.assertEqual(transcoding.type, 'video/mp4')
 		self.assertEqual(transcoding.url, 'http://another_url')
+
+	def test_does_not_create_video_when_providing_one(self):
+		transcoding = factories.create(
+			Transcoding,
+			quality='720p',
+			type='video/mp4',
+			url='http://another_url',
+			video=factories.create(Video, slug='yet-another-video-slug')
+		)
+
+		self.assertEquals(Video.objects.all().count(), 1)

From 019adcba56e4fe42a3415776bfddf095278d511f Mon Sep 17 00:00:00 2001
From: Bastiaan Welmers <bastiaan@welmers.net>
Date: Thu, 21 May 2020 18:37:41 +0200
Subject: [PATCH 3/7] Uploads are already 'lazily' executed

---
 tests/videodinges/factories.py                  |  4 +---
 tests/videodinges/test_factories/test_upload.py | 10 ++++++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/videodinges/factories.py b/tests/videodinges/factories.py
index 458bb89..4a09346 100644
--- a/tests/videodinges/factories.py
+++ b/tests/videodinges/factories.py
@@ -26,9 +26,7 @@ def create(model: Type[T], **kwargs) -> T:
 		return _create_with_defaults(models.Transcoding, kwargs, **defaults)
 
 	if model is models.Upload:
-		file = SimpleUploadedFile('some_file.txt', b'some contents') \
-			if 'file' not in kwargs else None
-		return _create_with_defaults(models.Upload, kwargs, file=file)
+		return _create_with_defaults(models.Upload, kwargs, file=SimpleUploadedFile('some_file.txt', b'some contents'))
 
 
 def _create_with_defaults(model: Type[T], kwargs: dict, **defaults) -> T:
diff --git a/tests/videodinges/test_factories/test_upload.py b/tests/videodinges/test_factories/test_upload.py
index a53603f..be67a83 100644
--- a/tests/videodinges/test_factories/test_upload.py
+++ b/tests/videodinges/test_factories/test_upload.py
@@ -1,3 +1,6 @@
+import os
+
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase
 
 from tests.videodinges import factories, UploadMixin
@@ -7,3 +10,10 @@ class UploadTestCase(UploadMixin, TestCase):
 	def test_model_is_created(self):
 		upload = factories.create(Upload)
 		self.assertEqual(upload.file.name, 'some_file.txt')
+		self.assertTrue(os.path.exists(os.path.join(self.media_root.name, 'some_file.txt')))
+
+	def test_upload_does_not_create_file_when_providing_upload(self):
+		upload = factories.create(Upload, file=SimpleUploadedFile('my_file.txt', b'some contents'))
+		self.assertEqual(upload.file.name, 'my_file.txt')
+		self.assertFalse(os.path.exists(os.path.join(self.media_root.name, 'some_file.txt')))
+		self.assertTrue(os.path.exists(os.path.join(self.media_root.name, 'my_file.txt')))

From 2e1c7e7165113819ca8990b740ce86f54f048651 Mon Sep 17 00:00:00 2001
From: Bastiaan Welmers <bastiaan@welmers.net>
Date: Thu, 21 May 2020 18:43:05 +0200
Subject: [PATCH 4/7] Raise NotImplementedError when unknown model is passed

---
 tests/videodinges/factories.py                  |  2 ++
 tests/videodinges/test_factories/test_create.py | 16 ++++++++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 tests/videodinges/test_factories/test_create.py

diff --git a/tests/videodinges/factories.py b/tests/videodinges/factories.py
index 4a09346..d9a7ead 100644
--- a/tests/videodinges/factories.py
+++ b/tests/videodinges/factories.py
@@ -28,6 +28,8 @@ def create(model: Type[T], **kwargs) -> T:
 	if model is models.Upload:
 		return _create_with_defaults(models.Upload, kwargs, file=SimpleUploadedFile('some_file.txt', b'some contents'))
 
+	raise NotImplementedError('Factory for %s not implemented' % model)
+
 
 def _create_with_defaults(model: Type[T], kwargs: dict, **defaults) -> T:
 	"""
diff --git a/tests/videodinges/test_factories/test_create.py b/tests/videodinges/test_factories/test_create.py
new file mode 100644
index 0000000..53b9ace
--- /dev/null
+++ b/tests/videodinges/test_factories/test_create.py
@@ -0,0 +1,16 @@
+from django.db import models
+
+from tests.videodinges import factories
+from django.test import TestCase
+
+class CreateTestCase(TestCase):
+
+	def test_factory_returns_model(self):
+
+		class NotImplementedModel(models.Model):
+			class Meta:
+				app_label = 'some_test_label'
+
+		with self.assertRaises(NotImplementedError):
+			factories.create(NotImplementedModel)
+

From c6f2bacf0fe83d23b8524e33fd562273122a3867 Mon Sep 17 00:00:00 2001
From: Bastiaan Welmers <bastiaan@welmers.net>
Date: Thu, 21 May 2020 19:08:59 +0200
Subject: [PATCH 5/7] Useful conditions can be put in lambdas

and an extra test to asure transcoding with upload is possible
---
 tests/videodinges/factories.py                | 13 +++++-----
 .../test_factories/test_transcoding.py        | 24 ++++++++++++++++---
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/tests/videodinges/factories.py b/tests/videodinges/factories.py
index d9a7ead..3f577b2 100644
--- a/tests/videodinges/factories.py
+++ b/tests/videodinges/factories.py
@@ -14,16 +14,17 @@ def create(model: Type[T], **kwargs) -> T:
 		return _create_with_defaults(models.Video, kwargs, title='Title', slug='slug', description='Description')
 
 	if model is models.Transcoding:
-		defaults = dict(
+		def url():
+			# only URL if no upload for they are mutually exclusive
+			if 'upload' not in kwargs:
+				return 'https://some_url'
+
+		return _create_with_defaults(models.Transcoding, kwargs,
 			video=lambda: create(models.Video),
 			quality=models.qualities[0].name,
 			type=str(models.transcoding_types[0]),
+			url=url,
 		)
-		if 'upload' not in kwargs:
-			# only URL if no upload for they are multually exclusive
-			defaults['url'] = 'https://some_url'
-
-		return _create_with_defaults(models.Transcoding, kwargs, **defaults)
 
 	if model is models.Upload:
 		return _create_with_defaults(models.Upload, kwargs, file=SimpleUploadedFile('some_file.txt', b'some contents'))
diff --git a/tests/videodinges/test_factories/test_transcoding.py b/tests/videodinges/test_factories/test_transcoding.py
index b1f691f..4e43b78 100644
--- a/tests/videodinges/test_factories/test_transcoding.py
+++ b/tests/videodinges/test_factories/test_transcoding.py
@@ -1,7 +1,9 @@
+import os
+
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase
-from videodinges.models import Transcoding, Video
-from tests.videodinges import factories
-from datetime import datetime
+from videodinges.models import Transcoding, Video, Upload
+from tests.videodinges import factories, UploadMixin
 
 class TranscodingTestCase(TestCase):
 	def test_factory_returns_model(self):
@@ -35,3 +37,19 @@ class TranscodingTestCase(TestCase):
 		)
 
 		self.assertEquals(Video.objects.all().count(), 1)
+
+
+class TranscodingWithUploadTestCase(UploadMixin, TestCase):
+	def test_can_assign_upload(self):
+		transcoding = factories.create(
+			Transcoding,
+			quality='720p',
+			type='video/mp4',
+			video=factories.create(Video, slug='yet-another-video-slug'),
+			upload=factories.create(Upload, file=SimpleUploadedFile('my_upload.txt', b'some_contents'))
+		)
+
+		self.assertTrue(os.path.exists(os.path.join(self.media_root.name, 'my_upload.txt')))
+		with open(os.path.join(self.media_root.name, 'my_upload.txt'), 'rb') as f:
+			self.assertEquals(f.read(), b'some_contents')
+

From c17b558d142a124e4d3ac18e4136bb92c66c4323 Mon Sep 17 00:00:00 2001
From: Bastiaan Welmers <bastiaan@welmers.net>
Date: Thu, 21 May 2020 19:34:02 +0200
Subject: [PATCH 6/7] Ability to generate unique fields in factories

---
 tests/videodinges/factories.py                | 23 +++++++++++++--
 .../test_factories/test_transcoding.py        |  2 +-
 .../videodinges/test_factories/test_video.py  | 29 +++++++++++++++++--
 tests/videodinges/views/test_video.py         |  2 +-
 4 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/tests/videodinges/factories.py b/tests/videodinges/factories.py
index 3f577b2..3196d43 100644
--- a/tests/videodinges/factories.py
+++ b/tests/videodinges/factories.py
@@ -1,4 +1,5 @@
 """ Module generating useful models in 1 place """
+from inspect import signature
 from typing import Type, TypeVar
 
 from django.core.files.uploadedfile import SimpleUploadedFile
@@ -11,7 +12,11 @@ T = TypeVar('T', bound=django.db.models.Model)
 
 def create(model: Type[T], **kwargs) -> T:
 	if model is models.Video:
-		return _create_with_defaults(models.Video, kwargs, title='Title', slug='slug', description='Description')
+		return _create_with_defaults(models.Video, kwargs,
+			title=lambda x: 'Title {}'.format(x),
+			slug=lambda x: 'slug-{}'.format(x),
+			description=lambda x: 'Description {}'.format(x),
+		)
 
 	if model is models.Transcoding:
 		def url():
@@ -38,6 +43,9 @@ def _create_with_defaults(model: Type[T], kwargs: dict, **defaults) -> T:
 		When providing lambda as default item, the result of the lambda will be taken.
 		The lambda will ONLY be executed when not provided in kwargs.
 
+		When a lambda requires an argument, the primary key of the to be created object
+		will be provided to that argument. This is useful for generating unique fields.
+
 		:param model: django model to create
 		:param kwargs: keyword arguments to fill the model
 		:param defaults: default keyword arguments to use when not mentioned in kwargs
@@ -45,6 +53,17 @@ def _create_with_defaults(model: Type[T], kwargs: dict, **defaults) -> T:
 
 	for k, v in defaults.items():
 		if callable(v) and not k in kwargs:
-			defaults[k] = v()
+			if len(signature(v).parameters) == 1:
+				result = v(_next_pk(model))
+			else:
+				result = v()
+			defaults[k] = result
 
 	return model.objects.create(**{**defaults, **kwargs})
+
+
+def _next_pk(model: Type[T]) -> int:
+	try:
+		return model.objects.order_by('-pk').first().pk + 1
+	except AttributeError:
+		return 1
diff --git a/tests/videodinges/test_factories/test_transcoding.py b/tests/videodinges/test_factories/test_transcoding.py
index 4e43b78..0f86df4 100644
--- a/tests/videodinges/test_factories/test_transcoding.py
+++ b/tests/videodinges/test_factories/test_transcoding.py
@@ -8,7 +8,7 @@ from tests.videodinges import factories, UploadMixin
 class TranscodingTestCase(TestCase):
 	def test_factory_returns_model(self):
 		transcoding = factories.create(Transcoding)
-		self.assertEqual(transcoding.video.slug, 'slug')
+		self.assertEqual(transcoding.video.slug, 'slug-1')
 		self.assertEqual(transcoding.quality, '360p')
 		self.assertEqual(transcoding.type, 'video/webm')
 		self.assertEqual(transcoding.url, 'https://some_url')
diff --git a/tests/videodinges/test_factories/test_video.py b/tests/videodinges/test_factories/test_video.py
index 3252a5e..ec4b957 100644
--- a/tests/videodinges/test_factories/test_video.py
+++ b/tests/videodinges/test_factories/test_video.py
@@ -6,8 +6,31 @@ from datetime import datetime
 class VideoTestCase(TestCase):
 	def test_factory_returns_model(self):
 		video = factories.create(Video)
-		self.assertEqual(video.slug, 'slug')
-		self.assertEqual(video.title, 'Title')
-		self.assertEqual(video.description, 'Description')
+		self.assertEqual(video.slug, 'slug-1')
+		self.assertEqual(video.title, 'Title 1')
+		self.assertEqual(video.description, 'Description 1')
 		self.assertIsInstance(video.created_at, datetime)
 		self.assertIsInstance(video.updated_at, datetime)
+
+	def test_factory_can_create_multiple_models(self):
+		video1 = factories.create(Video)
+		video2 = factories.create(Video)
+		video3 = factories.create(Video)
+
+		self.assertEqual(video1.slug, 'slug-1')
+		self.assertEqual(video1.title, 'Title 1')
+		self.assertEqual(video1.description, 'Description 1')
+		self.assertIsInstance(video1.created_at, datetime)
+		self.assertIsInstance(video1.updated_at, datetime)
+
+		self.assertEqual(video2.slug, 'slug-2')
+		self.assertEqual(video2.title, 'Title 2')
+		self.assertEqual(video2.description, 'Description 2')
+		self.assertIsInstance(video2.created_at, datetime)
+		self.assertIsInstance(video2.updated_at, datetime)
+
+		self.assertEqual(video3.slug, 'slug-3')
+		self.assertEqual(video3.title, 'Title 3')
+		self.assertEqual(video3.description, 'Description 3')
+		self.assertIsInstance(video3.created_at, datetime)
+		self.assertIsInstance(video3.updated_at, datetime)
diff --git a/tests/videodinges/views/test_video.py b/tests/videodinges/views/test_video.py
index 684b60a..aef91b8 100644
--- a/tests/videodinges/views/test_video.py
+++ b/tests/videodinges/views/test_video.py
@@ -70,7 +70,7 @@ class VideoTestCase(UploadMixin, TestCase):
 
 		self.assertInHTML('<h1>Vid 1</h1>', content)
 
-		self.assertInHTML('<p>Description</p>', content)
+		self.assertInHTML('<p>Description 1</p>', content)
 
 		self.assertInHTML('<strong>480p versie</strong>', content)
 

From 9f0b6881dd1d2a69234a7e1cf71c2de31cbf0fcb Mon Sep 17 00:00:00 2001
From: Bastiaan Welmers <bastiaan@welmers.net>
Date: Thu, 21 May 2020 20:00:01 +0200
Subject: [PATCH 7/7] Asure not too many queries are run

---
 tests/videodinges/factories.py                 | 12 ++++++++++--
 tests/videodinges/test_factories/test_video.py |  5 +++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/tests/videodinges/factories.py b/tests/videodinges/factories.py
index 3196d43..0867ab2 100644
--- a/tests/videodinges/factories.py
+++ b/tests/videodinges/factories.py
@@ -51,10 +51,18 @@ def _create_with_defaults(model: Type[T], kwargs: dict, **defaults) -> T:
 		:param defaults: default keyword arguments to use when not mentioned in kwargs
 	"""
 
+	_next_pk = 0
+	def next_pk():
+		# Queries next pk only one time during creation
+		nonlocal _next_pk
+		if _next_pk == 0:
+			_next_pk = _query_next_pk(model)
+		return _next_pk
+
 	for k, v in defaults.items():
 		if callable(v) and not k in kwargs:
 			if len(signature(v).parameters) == 1:
-				result = v(_next_pk(model))
+				result = v(next_pk())
 			else:
 				result = v()
 			defaults[k] = result
@@ -62,7 +70,7 @@ def _create_with_defaults(model: Type[T], kwargs: dict, **defaults) -> T:
 	return model.objects.create(**{**defaults, **kwargs})
 
 
-def _next_pk(model: Type[T]) -> int:
+def _query_next_pk(model: Type[T]) -> int:
 	try:
 		return model.objects.order_by('-pk').first().pk + 1
 	except AttributeError:
diff --git a/tests/videodinges/test_factories/test_video.py b/tests/videodinges/test_factories/test_video.py
index ec4b957..bc0c19d 100644
--- a/tests/videodinges/test_factories/test_video.py
+++ b/tests/videodinges/test_factories/test_video.py
@@ -34,3 +34,8 @@ class VideoTestCase(TestCase):
 		self.assertEqual(video3.description, 'Description 3')
 		self.assertIsInstance(video3.created_at, datetime)
 		self.assertIsInstance(video3.updated_at, datetime)
+
+	def test_factory_runs_only_2_queries(self):
+		""" Factory should only use 2 queries: one for selecting primary key, and one for inserting record """
+		with self.assertNumQueries(2):
+			video = factories.create(Video)