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.

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

Conversation

joshk
Copy link
Collaborator

@joshk joshk commented Sep 27, 2024

There are times when it's useful to see the log output when running tests, but we have disabled the logs.

I've brought back the default log settings while capturing logs from specific tests where errors and warnings are emitted within the functions being called.

@@ -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.

@jjcarstens
Copy link
Collaborator

I attached a commit with my recommendation to show what I was describing. I think this is the net-same behavior you were looking for without crazy log configuration and/or sleeps.

For example, here is a test failure with some logs injected in the code and they show up with the test results

image

Removes all the custom work of controlling logs and instead relies
on ExUnit tooling to capture and handle logs when they are useful.

This will keep the output clean unless a failure occurs and then it
will output the captured logs with the test
@jjcarstens jjcarstens force-pushed the allow-errors-and-warnings-in-test-env branch from 2dc5ba7 to aae2a37 Compare September 30, 2024 20:41
@joshk joshk merged commit 24640fb into main Oct 1, 2024
2 checks passed
@joshk joshk deleted the allow-errors-and-warnings-in-test-env branch October 1, 2024 02:21
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