5
11
Fork
You've already forked tomato
1

Remove associations from ShiftEvent & Shift factories #33

Merged
edwardloveall merged 10 commits from el-default-factories into main 2025年10月20日 15:13:26 +02:00

These associations create unnecessary data in the database. For example, the shift association in the ShiftEvent factory creates an additional shift_event, which means creating a shift_event from a factory actually creates 2. In addition, every shift_event creates one event. That means creating an event from its factory creates 3 events:

  • one from the event factory,
  • one from the shift event factory,
  • and one from the shift factory which creates an additional shift event

These extra associations sometimes cause tests to fail.

It seems like this was done to construct shifts, events, and shift_events more easily in tests. For example, ShiftEvent does not directly have an organization; Event does. But the factories were set up so you could assign those properties and the automatically created associations would use them.

This removes those extra associations now that the tests all use event factories.

This is a lot of reorganizing and I tried to split the commits up in a way that made them reasonable to review.

These associations create unnecessary data in the database. For example, the `shift` association in the `ShiftEvent` factory creates an additional `shift_event`, which means creating a `shift_event` from a factory actually creates 2. In addition, every `shift_event` creates one `event`. That means creating an `event` from its factory creates 3 events: - one from the event factory, - one from the shift event factory, - and one from the shift factory which creates an additional shift event These extra associations sometimes cause tests to fail. It seems like this was done to construct shifts, events, and shift_events more easily in tests. For example, `ShiftEvent` does not directly have an organization; `Event` does. But the factories were set up so you could assign those properties and the automatically created associations would use them. This removes those extra associations now that the tests all use `event` factories. This is a lot of reorganizing and I tried to split the commits up in a way that made them reasonable to review.
* `Taxonomy 9`
* `Taxonomy 10`
Would actually sort in the opposite order because `1` is before `9` in
a string. Switching to letters ensures that sorting by name (as the
`ExportService` does) will keep them in the order they were created in.
Technically, this could still cause a problem if the sequence gets
bigger by one letter:
* `Taxonomy ZZZZ`
* `Taxonomy AAAAA`
But that would require a little under half a million of these to be
generated in a single test run, so it's unlikely.
Remove associations from ShiftEvent & Shift factories
All checks were successful
Setup Successful
Jest Successful
Static Analysis Successful
RSpec Successful
28b3d45010
These associations create unnecessary data in the database. For
example, the `shift` association in the `ShiftEvent` factory creates an
additional `shift_event`, which means creating a `shift_event` from a
factory actually creates 2. In addition, every `shift_event` creates
one `event`. That means creating an `event` from a factory creates 3
events:
- one from the factory,
- one from the shift event,
- and one from the shift which creates an additional shift event
It seems like this was done to construct shifts, events, and
shift_events more easily in tests. For example, `ShiftEvent` does not
directly have an organization; `Event` does. But the factories were set
up so you could assign those properties and the automatically created
associations would use them.
This commit removes those extra associations now that the tests all use
`event` factories.
thetizzo left a comment
Copy link

This is awesome! Tough to catch something like this happening but the cleanup is great. I'm hoping it has a big effect on the amount of test flakes.

This is awesome! Tough to catch something like this happening but the cleanup is great. I'm hoping it has a big effect on the amount of test flakes.
edwardloveall deleted branch el-default-factories 2025年10月20日 15:13:27 +02:00
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
rootable/tomato!33
Reference in a new issue
rootable/tomato
No description provided.
Delete branch "el-default-factories"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?