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

Travis matrix #225

Merged
merged 1 commit into from
Mar 14, 2016
Merged

Travis matrix #225

merged 1 commit into from
Mar 14, 2016

Conversation

boyska
Copy link
Member

@boyska boyska commented Nov 22, 2015

Creates a "build matrix" which makes the different tests parallelizable and more "to the point" (ie: sphinx and flake8 are not installed when running unit tests)

refs #212

@boyska boyska added the tests label Nov 22, 2015
@boyska
Copy link
Member Author

boyska commented Mar 3, 2016

I think that this PR is still useful

@ael-code
Copy link
Member

ael-code commented Mar 4, 2016

We are going to enlarge the travis matrix a lot for testing multiple dependencies versions.
I really don't like the all-in-one-bash-file approach, because it hides and make the travis config more complex (both to read and modify).
Additionally I don't think that splitting tests at this level (flake, nose, docs) is so useful.

Anyway, If you really want to split tests we can try to use only the travis jaml file in order to keep things simple.

@boyska
Copy link
Member Author

boyska commented Mar 4, 2016

ael:

We are going to enlarge the travis matrix a lot for testing multiple dependencies versions.
I really don't like the all-in-one-bash-file approach, because it hides and make the travis config more complex (both to read and modify).

ok so I will try to put everything inside.

Additionally I don't think that splitting tests at this level (flake, nose, docs) is so useful.
Anyway, If you really want to split tests we can try to use only the travis jaml file in order to keep things simple.

ok, I'll do

boyska

@boyska
Copy link
Member Author

boyska commented Mar 4, 2016

Ok, done. By the way, I enabled "concurrent builds" on travis (I don't know why it wasn't) so now it is somewhat faster.

@ael-code
Copy link
Member

ael-code commented Mar 5, 2016

On 03/04/2016 04:23 PM, BoySka wrote:

Ok, done.

I like it much more now

By the way, I enabled "concurrent builds" on travis (I don't
know why it wasn't) so now it is somewhat faster.

Good

Anyway what about msgfmt?

@boyska
Copy link
Member Author

boyska commented Mar 5, 2016

ael:

Anyway what about msgfmt?

what do you mean? sorry, but I have not understood!

@ael-code
Copy link
Member

ael-code commented Mar 5, 2016

On 03/05/2016 11:11 PM, BoySka wrote:

ael:

Anyway what about msgfmt?

what do you mean? sorry, but I have not understood!

with this PR you've folded the imort msgfmt into a try-except block
and in case of import failure you will later print('msgfmt unavailable, skipping').

I was asking when it will be the case that the import msgfmt will fails.

@boyska
Copy link
Member Author

boyska commented Mar 6, 2016

You're right, I don't know why I did that, but now I removed it and everything seems to work fine.

@boyska boyska mentioned this pull request Mar 6, 2016
@ael-code
Copy link
Member

ael-code commented Mar 6, 2016

waiting for review on boyska#2

@ael-code
Copy link
Member

ael-code commented Mar 7, 2016

On travis there is a job related to no-envvars case that perform only the installation

@boyska
Copy link
Member Author

boyska commented Mar 7, 2016

ael:

On travis there is a job related to no-envvars case that perform only
the installation

you're right but... can you understand why?

@ael-code
Copy link
Member

ael-code commented Mar 8, 2016

On my PR this do not happen: https://travis-ci.org/ael-code/libreant/builds/114050261.

As stated in the official travis docs about matrix.include:

This adds a particular job to the build matrix which has already been populated.

This is useful if you want to only test the latest version of a dependency together with the latest version of the runtime.

So you've populated the matrix with python2.7 job and then you are adding other stuff with matrix.includes

I think I've the solution for this problem, but since we have a lot of PR opened, I would like to proceeds as follows:

  • merging this PR without the matrix.includes directive that is useless at this stage
  • I will propose my solution on the PR Travis split testing #257, where the matrix.includes matters.

@boyska
Copy link
Member Author

boyska commented Mar 8, 2016

ael:

As stated in the official travis docs about matrix.include:

This adds a particular job to the build matrix which has already been populated.

oh, that's nice, they are using a declarative format as if it was an
imperative language. gotcha.

This is useful if you want to only test the latest version of a dependency together with the latest version of the runtime.

So you've populated the matrix with python2.7 job and then you are adding other stuff with matrix.includes

I think I've the solution for this problem, but since we have a lot of PR opened, I would like to proceeds as follows:

  • merging this PR without the matrix.includes directive that is useless at this stage

ok, I took the commit you proposed in a PR on my repo (could net accept
it because of rebase issues). You should be now willing to merge this
PR, and close the other one.

boyska

@ael-code
Copy link
Member

ael-code commented Mar 8, 2016

rebase?

@boyska boyska force-pushed the travis-matrix branch 3 times, most recently from 2f8f354 to 91c46d6 Compare March 9, 2016 01:11
@boyska
Copy link
Member Author

boyska commented Mar 9, 2016

I'm reintroducing matrix-include, since the tests with multiple gevent versions are already in.
I know that you will get mad at this :) but I really don't know how that could be handled differently. If you have any idea, please go on.

@ael-code
Copy link
Member

ael-code commented Mar 9, 2016

I've used your inline patter in another project and I discovered that is wrong :)

[[ $TEST_SUITE == flake ]] && pip install flake8 || true

when pip install flake8 fails, true will be returned.

We should move to something like:

if [[ $TEST_SUITE == flake ]] ; then pip install flake8; fi

@ael-code
Copy link
Member

ael-code commented Mar 9, 2016

We should move to something like:

if [[ $TEST_SUITE == flake ]] ; then pip install flake8; fi

Forget it. Also my solution is wrong.

@ael-code
Copy link
Member

ael-code commented Mar 9, 2016

oke... This pattern should work:

if [[ $ENV_NAME == env ]] ; then <command>; return $?; fi

I did not find anything shorter

@boyska
Copy link
Member Author

boyska commented Mar 9, 2016

you're right, I will fix it asap

@boyska
Copy link
Member Author

boyska commented Mar 9, 2016

no, wait; does the return thing really work? this is not valid bash syntax on my computer!

@boyska boyska force-pushed the travis-matrix branch 2 times, most recently from c8f2f03 to b83561d Compare March 10, 2016 00:03
@boyska
Copy link
Member Author

boyska commented Mar 11, 2016

Assuming that it works, I have used it.

@ael-code
Copy link
Member

Assuming that it works, I have used it.

I think that it will be better if we use the if-return pattern everywhere. For example we are interested also in the exit code of python setup install

ael-code added a commit that referenced this pull request Mar 14, 2016
@ael-code ael-code merged commit 70bb7cc into insomnia-lab:dev Mar 14, 2016
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.

2 participants