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

Test on more elixir versions #74

Merged
merged 1 commit into from
Mar 10, 2017
Merged

Test on more elixir versions #74

merged 1 commit into from
Mar 10, 2017

Conversation

gmile
Copy link
Contributor

@gmile gmile commented Aug 10, 2016

No description provided.

@coveralls
Copy link

coveralls commented Aug 10, 2016

Coverage Status

Coverage remained the same at 77.536% when pulling aeb723b on gmile:test-on-more-ex-versions into 7a97254 on robconery:master.

@gmile
Copy link
Contributor Author

gmile commented Aug 10, 2016

Looks like not all elixir/erlang combinations (apart from obvious ones, like elixir 1.x and erlang 19.0) are supported. I'll look closer at what's going on.

@coveralls
Copy link

coveralls commented Aug 10, 2016

Coverage Status

Coverage remained the same at 77.256% when pulling 66b0569 on gmile:test-on-more-ex-versions into 61cf49d on robconery:master.

@coveralls
Copy link

coveralls commented Aug 10, 2016

Coverage Status

Coverage remained the same at 77.256% when pulling decc38a on gmile:test-on-more-ex-versions into 61cf49d on robconery:master.

@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage remained the same at 77.256% when pulling 942daf6 on gmile:test-on-more-ex-versions into 61cf49d on robconery:master.

@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage remained the same at 77.256% when pulling bd948c3 on gmile:test-on-more-ex-versions into 61cf49d on robconery:master.

@gmile
Copy link
Contributor Author

gmile commented Aug 11, 2016

First of all, the issues are not related to the code, they are related to the build. In one case, a test using mock fails. In another case, coverall fails.

So two issues block this PR at the moment:

  1. Argument error when running mix coveralls on project parroty/excoveralls#60 – this is the biggest one right now. We'll have to wait until this is resolved.

  2. issue with mock (actually very likely with undelying meck) (as seen here, for instance):

    ** (EXIT from #PID<0.2313.0>) {:compile_forms, {:error, [{'lib/system.ex', [{235, :erl_lint, {:type_syntax, :integer}}]}], [{'lib/system.ex', [{0, :erl_lint, {:unused_var, :cwd@1}}]}]}}
    

    I barely understand the mocking code in meck. Trying to fix this upstream may be a waste of time. I'd rather have the test removed, and claim support for 1.1.1. Or remove support fo 1.1.1, and add it only on popular demand.

@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage remained the same at 77.256% when pulling 66e62b5 on gmile:test-on-more-ex-versions into 61cf49d on robconery:master.

@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage remained the same at 77.256% when pulling 66e62b5 on gmile:test-on-more-ex-versions into 61cf49d on robconery:master.

Copy link
Contributor

@joshsmith joshsmith left a comment

Choose a reason for hiding this comment

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

Quick question here:

.travis.yml Outdated
sudo: false
env:
- MIX_ENV=test STRIPE_SECRET_KEY=non_empty_secret_key_string
script: mix coveralls.travis
after_script:
- mix inch.report
exclude:
- otp_release: 19.0
elixir: 1.1.1
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the exclude doing here when used in combination with those elixir and otp_release above?

Copy link
Contributor Author

@gmile gmile Oct 19, 2016

Choose a reason for hiding this comment

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

@joshsmith Travis builds a matrix with values from otp_releases as rows and values from elixir as columns. Then it runs the test script for each "cell", e.g. a pair of elixir/otp_release.

Using exclude we can "cross out" cells for which we don't want Travis to run the test script.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gmile I guess my question though is whether we want exclude when we're listing it above? Wouldn't we just not list it above and then exclude isn't necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshsmith I think elixir 1.1.x doesn't work with erlang 19.0, that's why it was listed

@gmile
Copy link
Contributor Author

gmile commented Oct 24, 2016

Btw this PR now fails on Travis, since the library is no longer under robconnery on Travis:

image

@joshsmith
Copy link
Contributor

@gmile would you mind rebasing this onto latest?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.049% when pulling 3e6a3cf on gmile:test-on-more-ex-versions into f00f396 on code-corps:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.049% when pulling 3e6a3cf on gmile:test-on-more-ex-versions into f00f396 on code-corps:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.049% when pulling 3e6a3cf on gmile:test-on-more-ex-versions into f00f396 on code-corps:master.

@coveralls
Copy link

coveralls commented Mar 10, 2017

Coverage Status

Coverage remained the same at 78.049% when pulling 6d33188 on gmile:test-on-more-ex-versions into f00f396 on code-corps:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.049% when pulling 6d33188 on gmile:test-on-more-ex-versions into f00f396 on code-corps:master.

@coveralls
Copy link

coveralls commented Mar 10, 2017

Coverage Status

Coverage remained the same at 78.049% when pulling 6d33188 on gmile:test-on-more-ex-versions into f00f396 on code-corps:master.

@gmile
Copy link
Contributor Author

gmile commented Mar 10, 2017

Hey @joshsmith, I just rebased the branch. Sorry it took a while.

@joshsmith
Copy link
Contributor

Hey @gmile no worries at all! I think I'm going to squash, too. I hadn't realized before that I can do this myself if you enable me to do so in your fork.

@joshsmith
Copy link
Contributor

@gmile reached out on the Elixir slack, but I can't actually update your branch. Would you be able to squash down to one commit or give me access to your fork to do so? Thanks!

@gmile
Copy link
Contributor Author

gmile commented Mar 10, 2017

@joshsmith added you as collaborator of my fork, simply because that was quicker to do :)

@coveralls
Copy link

coveralls commented Mar 10, 2017

Coverage Status

Coverage remained the same at 78.049% when pulling 90dfe2d on gmile:test-on-more-ex-versions into f00f396 on code-corps:master.

@joshsmith joshsmith merged commit 224cea4 into beam-community:master Mar 10, 2017
@joshsmith joshsmith deleted the test-on-more-ex-versions branch March 10, 2017 20:15
@joshsmith
Copy link
Contributor

🙌 thanks for everything you did working through this. A lot of work for +9 LOC, but I know the work that went into it, so 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants