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

Improve attachment support detection #567

Conversation

sasa1977
Copy link
Contributor

@sasa1977 sasa1977 commented Dec 1, 2020

This fixes a subtle bug which we encountered on one project in tests.

The problem is that function_exported? returns false if the module is not loaded (see here). Consequently, if the beam is started in interactive mode (default when mix is used), Bamboo may incorrectly conclude that attachments are not supported.

This can lead to some strange behaviour in tests. For example, we had a test failing if executed in isolation, but sometimes the test would pass if all the tests are running. The test would always pass if the test adapter is recompiled while invoking mix test.

Note that the same issue can occur in prod if OTP release is not used.

This fixes a subtle bug which we encountered on one project in tests.

The problem is that `function_exported?` returns false if the module is not loaded (see [here](https://hexdocs.pm/elixir/Kernel.html?#function_exported?/3)). Consequently, if the beam is started in interactive mode (default when mix is used), Bamboo may incorrectly conclude that attachments are not supported.

This can lead to some strange behaviour in tests. For example, we had a test failing if executed in isolation, but sometimes the test would pass if all the tests are running. The test would always pass if the test adapter is recompiled while invoking `mix test`.

Note that the same issue can occur in prod if OTP release is not used.
Copy link
Collaborator

@germsvel germsvel left a comment

Choose a reason for hiding this comment

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

Thanks for catching this error and opening a PR @sasa1977 .

Looks great 🎉

@germsvel germsvel merged commit 4626eea into beam-community:master Dec 1, 2020
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