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.

Sign up for GitHub

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

Fixed some factory issues to address Percy's false positive flags #11771

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Fixed some factory issues to address Percy's false positive flags #11771

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5cd4015
fixed how to factory generate dates so they are relative to a fixed d…
mmmavis Jan 25, 2024
6b53875
fixed how factory generates MozFest's ticket snippet's cost value
mmmavis Jan 25, 2024
dac1db5
removed an unused import
mmmavis Jan 25, 2024
6323908
Merge branch 'main' into 10328-percy-flags-date
mmmavis Jan 25, 2024
d6fc39e
fixed linitng error
mmmavis Jan 25, 2024
e156704
Merge branch '10328-percy-flags-date' of https://github.com/mozilla/f…
mmmavis Jan 25, 2024
54e1212
typo...
mmmavis Jan 26, 2024
c5cf7b9
updated fixed values used in Playwright tests
mmmavis Jan 26, 2024
df4517d
Merge branch 'main' into 10328-percy-flags-date
mmmavis Jan 31, 2024
245ac11
Merge branch 'main' into 10328-percy-flags-date
mmmavis Apr 22, 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
11 changes: 3 additions & 8 deletions network-api/networkapi/highlights/factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from datetime import timezone
from datetime import datetime, timezone

from factory import Faker, LazyAttribute, Trait, post_generation
from factory import Faker, LazyAttribute, post_generation
from factory.django import DjangoModelFactory
from wagtail_factories import ImageFactory

Expand All @@ -18,17 +18,12 @@ class Meta:
"footer_sentence",
)

class Params:
unpublished = Trait(publish_after=Faker("future_datetime", end_date="+30d", tzinfo=timezone.utc))
Copy link
Collaborator Author

@mmmavis mmmavis Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

These details seem a bit unnecessary to me. I'm deleting them to avoid complication.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are helpful if creating these in a test. Instead of doing something like HighlightFactory(publish_after=timedelta(datetime.now() + 1)) to indicate an unpublished highlight, we can just do HighlightFactory(unpublished=True) and it will create it for us with a publish_after in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's worth deleting them, they are helpful factory traits

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cool I'll add them back!

has_expiry = Trait(expires=Faker("future_datetime", end_date="+30d", tzinfo=timezone.utc))
expired = Trait(expires=Faker("past_datetime", start_date="-30d", tzinfo=timezone.utc))

title = LazyAttribute(lambda o: o.title_sentence.rstrip("."))
description = Faker("paragraph", nb_sentences=5, variable_nb_sentences=True)
link_label = LazyAttribute(lambda o: " ".join(o.link_label_words))
footer = LazyAttribute(lambda o: o.footer_sentence.rstrip("."))
link_url = Faker("uri")
publish_after = Faker("past_datetime", start_date="-30d", tzinfo=timezone.utc)
publish_after = Faker("past_datetime", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)
expires = None
Comment on lines -31 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how this would solve the problem though... 🤔 we would still get a random date but now between Jan 1st 2020 and the current date instead of the last 30 days. We might need a different approach

order = 0

Expand Down
4 changes: 1 addition & 3 deletions network-api/networkapi/mozfest/factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import random

from django.conf import settings
from factory import Faker, LazyAttribute, SubFactory
from factory.django import DjangoModelFactory
Expand Down Expand Up @@ -87,7 +85,7 @@ class Meta:
model = mozfest_models.Ticket

name = Faker("sentence", nb_words=2)
cost = f"€{random.choice(['100', '200', '300'])}"
cost = Faker("random_element", elements=(("€100", "€200", "€300")))
group = Faker("text", max_nb_chars=50)
description = Faker("text", max_nb_chars=250)
event = SubFactory(events_factory.TitoEventFactory)
Expand Down
25 changes: 3 additions & 22 deletions network-api/networkapi/news/factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from datetime import timezone
from datetime import datetime, timezone
from random import shuffle

from django.conf import settings
Expand Down Expand Up @@ -32,34 +32,15 @@ class Meta:
exclude = ("headline_sentence",)

class Params:
unpublished = Trait(
publish_after=Faker("date_time", tzinfo=timezone.utc)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Similarly, these details seem a bit unnecessary to me. I'm deleting them to avoid complication.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to keep these as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will add them back!

if RANDOM_SEED and not TESTING
else Faker("future_datetime", end_date="+30d", tzinfo=timezone.utc)
)
has_expiry = Trait(
expires=Faker("date_time", tzinfo=timezone.utc)
if RANDOM_SEED and not TESTING
else Faker("future_datetime", end_date="+30d", tzinfo=timezone.utc)
)
expired = Trait(
expires=Faker("date_time", tzinfo=timezone.utc)
if RANDOM_SEED and not TESTING
else Faker("past_datetime", start_date="-30d", tzinfo=timezone.utc)
)
video = Trait(is_video=True)

headline = LazyAttribute(lambda o: o.headline_sentence.rstrip("."))
outlet = Faker("company")
date = Faker("date") if RANDOM_SEED and not TESTING else Faker("past_date", start_date="-30d")
date = Faker("past_date", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)
link = Faker("url")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm unsure why we needed to have two ways to generate values for date here. Any insights?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure... it seems to be generating a random date in the past 30 days for testing while for all other generations it uses a random date with no restrictions

excerpt = Faker("paragraph", nb_sentences=3, variable_nb_sentences=True)
author = Faker("name")
publish_after = (
Faker("date_time", tzinfo=timezone.utc)
if RANDOM_SEED and not TESTING
else Faker("past_datetime", start_date="-30d", tzinfo=timezone.utc)
)
publish_after = Faker("past_datetime", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same question here. I'm curious why RANDOM_SEED and TESTING matter when it comes to generating value for publish_after

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure either. I think we're safe to simplify this if the Python tests pass.

# LazyAttribute helper value
headline_sentence = Faker("sentence", nb_words=4)
Expand Down
8 changes: 2 additions & 6 deletions network-api/networkapi/wagtailpages/factory/blog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from datetime import timezone
from datetime import datetime, timezone
from random import choice

from django.conf import settings
Expand Down Expand Up @@ -88,11 +88,7 @@ class Meta:

title = LazyAttribute(lambda o: o.title_text.rstrip("."))
body = Faker("streamfield", fields=blog_body_streamfield_fields)
first_published_at = (
Faker("date_time", tzinfo=timezone.utc)
if RANDOM_SEED and not TESTING
else Faker("past_datetime", start_date="-30d", tzinfo=timezone.utc)
)
first_published_at = Faker("past_datetime", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)
search_description = Faker("paragraph", nb_sentences=5, variable_nb_sentences=True)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same q as above.

Copy link
Contributor

Choose a reason for hiding this comment

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

See above

live = True

Expand Down
10 changes: 5 additions & 5 deletions network-api/networkapi/wagtailpages/factory/buyersguide.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ class Meta:
blurb = Faker("sentence")
product_url = Faker("url")
worst_case = Faker("sentence")
first_published_at = Faker("past_datetime", start_date="-2d", tzinfo=timezone.utc)
last_published_at = Faker("past_datetime", start_date="-1d", tzinfo=timezone.utc)
first_published_at = Faker("past_datetime", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)
last_published_at = Faker("date_time_between", start_date=first_published_at, tzinfo=timezone.utc)
evaluation = SubFactory("networkapi.wagtailpages.factory.buyersguide.ProductPageEvaluationFactory")
locale = LazyFunction(lambda: Locale.get_default())

Expand Down Expand Up @@ -246,7 +246,7 @@ class Meta:

title = Faker("sentence")
hero_image = SubFactory(ImageFactory)
first_published_at = Faker("past_datetime", start_date="-30d", tzinfo=timezone.utc)
first_published_at = Faker("past_datetime", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)
search_image = SubFactory(ImageFactory)
search_description = Faker("paragraph", nb_sentences=5, variable_nb_sentences=True)
body = Faker(
Expand All @@ -271,7 +271,7 @@ class Meta:
header = Faker("sentence")
title = Faker("sentence")
cta = SubFactory(PetitionFactory)
first_published_at = Faker("past_datetime", start_date="-30d", tzinfo=timezone.utc)
first_published_at = Faker("past_datetime", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)
narrowed_page_content = Faker("boolean", chance_of_getting_true=50)
body = Faker(
provider="streamfield",
Expand All @@ -292,7 +292,7 @@ class Meta:

year = Faker("random_element", elements=(("2023", "2023"))) # Add extra years here once available
title = LazyAttribute(lambda o: f"Annual Consumer Creepometer {o.year}")
first_published_at = Faker("past_datetime", start_date="-30d", tzinfo=timezone.utc)
first_published_at = Faker("past_datetime", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)
search_image = SubFactory(ImageFactory)
search_description = Faker("paragraph", nb_sentences=5, variable_nb_sentences=True)

Expand Down
10 changes: 3 additions & 7 deletions network-api/networkapi/wagtailpages/factory/publication.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from datetime import timezone
from datetime import datetime, timezone
from random import randint, random, shuffle

from django.conf import settings
Expand Down Expand Up @@ -27,7 +27,7 @@ class PublicationPageFactory(PageFactory):
title = Faker("text", max_nb_chars=120)
subtitle = Faker("text", max_nb_chars=250)
secondary_subtitle = Faker("text", max_nb_chars=250)
publication_date = Faker("date_object")
first_published_at = Faker("past_datetime", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)
hero_image = SubFactory(ImageFactory)
publication_file = SubFactory(DocumentFactory)
intro_notes = Faker("sentence")
Expand All @@ -53,11 +53,7 @@ class Meta:
publication_date = Faker("date_object")
article_file = SubFactory(DocumentFactory)
body = Faker("streamfield", fields=article_body_streamfield_fields)
first_published_at = (
Faker("date_time", tzinfo=timezone.utc)
if RANDOM_SEED and not TESTING
else Faker("past_datetime", start_date="-30d", tzinfo=timezone.utc)
)
first_published_at = Faker("past_datetime", start_date=datetime(2020, 1, 1), tzinfo=timezone.utc)
search_description = Faker("paragraph", nb_sentences=5, variable_nb_sentences=True)
live = True

Expand Down
4 changes: 2 additions & 2 deletions tests/integration/pni/search.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ test.describe("PNI search", () => {
health: 11,
smart: 4,
percy: 1,
searchTerm: 3,
searchTermWithDing: 2,
searchTerm: 5,
searchTermWithDing: 4,
};

const qs = {
Expand Down
Loading