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

Allow errors and warnings in test env #1561

Merged
merged 4 commits into from
Oct 1, 2024
Merged

Allow errors and warnings in test env #1561

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e468b1
Add the default log settings for `test` env back
joshk Sep 27, 2024
339a394
Ignore the logs from some tests which create errors and warnings
joshk Sep 27, 2024
788da0b
Allows ws connections to close cleaning in `channel_case.ex`
joshk Sep 27, 2024
aae2a37
Capture logs by default
jjcarstens Sep 30, 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
3 changes: 2 additions & 1 deletion config/test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import Config

config :bcrypt_elixir, log_rounds: 4

config :logger, :default_handler, false
# Print only warnings and errors during test
config :logger, level: :warning

##
# NervesHub Web
Expand Down
3 changes: 3 additions & 0 deletions test/nerves_hub/firmwares_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ defmodule NervesHub.FirmwaresTest do
end

describe "create_firmware/2" do
@tag capture_log: true
test "remote creation failure triggers transaction rollback", %{
org: org,
org_key: org_key,
Expand All @@ -50,6 +51,7 @@ defmodule NervesHub.FirmwaresTest do
assert ^firmwares = Firmwares.get_firmwares_by_product(product.id)
end

@tag capture_log: true
test "enforces uuid uniqueness within a product",
%{firmware: %{upload_metadata: %{local_path: filepath}}, org: org} do
assert {:error, %Ecto.Changeset{errors: [uuid: {"has already been taken", [_ | _]}]}} =
Expand Down Expand Up @@ -287,6 +289,7 @@ defmodule NervesHub.FirmwaresTest do
Firmwares.get_firmware_delta_by_source_and_target(source, target)
end

@tag capture_log: true
test "new firmware delta is not created if there is an error", %{
firmware: source,
org_key: org_key,
Expand Down
2 changes: 2 additions & 0 deletions test/nerves_hub_web/channels/device_channel_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ defmodule NervesHubWeb.DeviceChannelTest do
end

describe "unhandled messages are caught" do
@tag capture_log: true
test "handle_info" do
user = Fixtures.user_fixture()
{device, _firmware, _deployment} = device_fixture(user, %{identifier: "123"})
Expand All @@ -317,6 +318,7 @@ defmodule NervesHubWeb.DeviceChannelTest do
assert_connection_change()
end

@tag capture_log: true
test "handle_in" do
user = Fixtures.user_fixture()
{device, _firmware, _deployment} = device_fixture(user, %{identifier: "123"})
Expand Down
1 change: 1 addition & 0 deletions test/nerves_hub_web/live/devices/show_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ defmodule NervesHubWeb.Live.Devices.ShowTest do
assert after_audit_count == before_audit_count + 1
end

@tag capture_log: true
test "reboot blocked", %{conn: conn, fixture: fixture} do
Repo.preload(fixture.user, :org_users)
|> Map.get(:org_users)
Expand Down
3 changes: 3 additions & 0 deletions test/nerves_hub_web/live/firmware_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ defmodule NervesHubWeb.Live.FirmwareTest do
|> assert_has("h1", text: "Firmware")
end

@tag capture_log: true
test "error if corrupt firmware uploaded", %{
conn: conn,
user: user,
Expand Down Expand Up @@ -176,6 +177,7 @@ defmodule NervesHubWeb.Live.FirmwareTest do
|> assert_has("div", text: "Firmware corrupt, signature invalid, or missing public key")
end

@tag capture_log: true
test "error if org keys do not match firmware", %{
conn: conn,
user: user,
Expand Down Expand Up @@ -210,6 +212,7 @@ defmodule NervesHubWeb.Live.FirmwareTest do
|> assert_has("div", text: "Firmware corrupt, signature invalid, or missing public key")
end

@tag capture_log: true
test "error if meta-product does not match product name", %{
conn: conn,
user: user,
Expand Down
3 changes: 3 additions & 0 deletions test/support/channel_case.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ defmodule NervesHubWeb.ChannelCase do
pid = Ecto.Adapters.SQL.Sandbox.start_owner!(NervesHub.Repo, shared: not tags[:async])
on_exit(fn -> Ecto.Adapters.SQL.Sandbox.stop_owner(pid) end)

# This extra sleep fixes Ecto ownership errors in the logs
on_exit(fn -> Process.sleep(10) end)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a hard time adding any sleeps to test. This is essentially 10 * channel test count. It's seems you're adding to avoid some annoying logs but I think we could change the approach to solve it as well

Instead of capturing logs for a few tests, we enable logging, but then at capture_log: true to the start function of test_helper.exs for all tests. I believe the ExUnit behavior is to print the logs it captured when test fails which would give us the useful output only when needed.

If that's not the case then I guess I'll need to help figure out how to get rid of the annoyance and not have sleeps 😉

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 can totally understand the feeling that adding sleeps to tests is a code smell.

I think it might be best to show these errors in a Zoom and see if there are other options available.


:ok
end
end
Loading