Skip to content

Eliminate Process.sleep from integration tests - #6830

Merged
SteffenDE merged 1 commit into
phoenixframework:mainfrom
praialabs:integration-test-no-sleep
Sep 4, 2026
Merged

Eliminate Process.sleep from integration tests#6830
SteffenDE merged 1 commit into
phoenixframework:mainfrom
praialabs:integration-test-no-sleep

Conversation

@rhcarvalho

Copy link
Copy Markdown
Contributor

Previously, consecutive generator commands in integration tests (such as phx.gen.auth followed by phx.gen.live or multiple scoped generators) invoked Process.sleep(1500) to ensure that newly generated migrations received distinct 1-second timestamp versions.

In aggregate across all tests, these blind sleeps introduced unnecessary idle latency to test runs.

This adds adjust_migration_timestamps/1:

  • Adjusts timestamps of existing migration files on disk to deterministic, sequentially ordered timestamps in the past.
  • Allows subsequent generators to produce fresh, non-colliding migration versions immediately without sleeping.
  • Removes all 8 instances of Process.sleep(1500) across integration tests.

@rhcarvalho
rhcarvalho marked this pull request as draft September 2, 2026 20:06
@rhcarvalho

Copy link
Copy Markdown
Contributor Author

I don't expect the gain to be significant here, but in general Process.sleep is not great when we have better alternatives, and those calls showed up as I work on other integration test improvements.

Previously, consecutive generator commands in integration tests (such as
`phx.gen.auth` followed by `phx.gen.live` or multiple scoped generators)
invoked `Process.sleep(1500)` to ensure that newly generated migrations
received distinct 1-second timestamp versions.

In aggregate across all tests, these blind sleeps introduced unnecessary
idle latency to test runs.

This adds `adjust_migration_timestamps/1`:

- Adjusts timestamps of existing migration files on disk to
  deterministic, sequentially ordered timestamps in the past.
- Allows subsequent generators to produce fresh, non-colliding migration
  versions immediately without sleeping.
- Removes all 8 instances of `Process.sleep(1500)` across integration
  tests.
@rhcarvalho
rhcarvalho force-pushed the integration-test-no-sleep branch from 96e8485 to a003160 Compare September 2, 2026 20:20
@rhcarvalho
rhcarvalho marked this pull request as ready for review September 2, 2026 22:01
Comment on lines +120 to +125
[
Path.join(app_path, "priv/repo/migrations/*_*.exs"),
Path.join(app_path, "apps/*/priv/repo/migrations/*_*.exs")
]
|> Enum.flat_map(&Path.wildcard/1)
|> Enum.with_index()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am wondering, is it guaranteed that those files will always be in order?

@rhcarvalho rhcarvalho Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I had a Enum.sort() there, and had the same question when I was reviewing. After some research I decided to drop it.

Neither https://elixir.hexdocs.pm/1.20.4/Path.html#wildcard/2 nor https://www.erlang.org/doc/apps/stdlib/filelib.html#wildcard/1 explicitly mention sort order. The Erlang docs compare it to "Unix-style" where sort order is one of the guarantees.

Not sure what's the best linkable reference, but I found https://pubs.opengroup.org/onlinepubs/007904975/functions/glob.html#:~:text=The%20pathnames%20shall%20be%20in%20sort%20order%20as%20defined%20by%20the%20current%20setting%20of%20the%20LC_COLLATE%20category

The Erlang implementation sorts the output for at least 17 years, probably since it was introduced:

https://github.com/erlang/otp/blame/OTP-29.0.6/lib/stdlib/src/filelib.erl#L435-L444

@SteffenDE
SteffenDE merged commit 1e6183e into phoenixframework:main Sep 4, 2026
9 checks passed
@rhcarvalho
rhcarvalho deleted the integration-test-no-sleep branch September 4, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants