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

ignore_sigterm.sh: Put date after sleep #35

Closed
wants to merge 1 commit into from

Conversation

lanodan
Copy link
Contributor

@lanodan lanodan commented Jan 22, 2024

At least the only way for test/exile/process_test.exs:225 to work correctly
is to use sleep(1) as date and echo barely take any time to execute.

At least the only way for test/exile/process_test.exs:225 to work correctly
is to use sleep(1) as date and echo barely take any time to execute.
@akash-akya
Copy link
Owner

Hi @lanodan, can you share what issue you were facing with this test? It was failing for BSD systems is it?

@lanodan
Copy link
Contributor Author

lanodan commented Jan 23, 2024

Nah, it's on linux. Reading the test case again it doesn't seems to be timeout-related as it fails right before.

This is the test failure I reliably get:

mix test test/exile/process_test.exs:225
Excluding tags: [:test]
Including tags: [line: "225"]

  1) test process termination await_exit with timeout (Exile.ProcessTest)
     test/exile/process_test.exs:225
     match (=) failed
     code:  assert {:ok, "ignored signals\n"} = Process.read(s)
     left:  {:ok, "ignored signals\n"}
     right: {:ok, "ignored signals\n2024-01-24_00:11:37\n"}
     stacktrace:
       test/exile/process_test.exs:230: (test)

Maybe the assert should be changed instead? I'm not really sure what's this assert is supposed to test.

@akash-akya
Copy link
Owner

akash-akya commented Jan 24, 2024

Understood. This assertion is to ensure that script is started and trap is called (aka synchronize). Before we actually call await_exit.
We don't really care if the output contains date or not. I will push changes to relax the assertion.

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