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

Fix compilation errors #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

randycoulman
Copy link

Fixes #22
Replaces #24

From my reading of the docs and other research, it looks like Code.ensure_loaded? is a better way to check whether an optional dependency module exists or not. Details below. This PR preserves the intention of only including Absinthe-related code when Absinthe is present in the parent project. #24 was a temporary fix to disable the check as I understand it.

This also fixes a formatting-related test failure in the doc tests for assertions/absinthe.ex. I still get 3 test failures in the doc tests (with Elixir 1.12.3), but I'm not sure what the problem is or how to fix them. The errors all happen in format_fields with the message:

** (Protocol.UndefinedError) protocol Enumerable not implemented for :scalar of type Atom. This protocol is implemented for the following type(s): HashSet, Range, Map, Function, List, Stream, Date.Range, HashDict, GenEvent.Stream, MapSet, File.Stream, IO.Stream

Details:

From my reading of the docs and other research, it looks like `Code.ensure_loaded?` is a better way to check whether an optional dependency module exists or not.
@darrenklein
Copy link

@devonestes this seems to do the trick, would you consider accepting this PR?

The problem of the three failing doctests is unrelated to this particular issue - I think I see the problem there, will try to experiment with that in the next day or two.

@darrenklein
Copy link

@randycoulman I patched up that failing doctest issue in another PR, thanks for bringing it up!

@darrenklein
Copy link

@devonestes just a friendly ping, thank you!

@mattste
Copy link

mattste commented May 13, 2022

I wasn't able to get this P/R to work when doing a fresh install. The error originally popped-up in CI when a fresh install was done. I can't track down a specific reason why but Assertions is being compiled before Absinthe is available. The Code.ensure_loaded? check always returns false.

My workaround for now is a fork that removes the check.

mattste pushed a commit to Virtual-Repetitions/assertions that referenced this pull request May 13, 2022
See this issue and comment: devonestes#27 (comment)
@rizasal
Copy link

rizasal commented Nov 7, 2022

@mattste
Marking the absinthe dependency as optional: true in the mix.exs file helped fix the issue you've mentioned.
Currently using a fork that combines #27, #29 and the above change.
https://github.com/rizasal/assertions

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.

Can't seem to properly import Absinthe assertions
4 participants