lambdaclass / erlings

Small exercises to get you used to reading and writing Erlang code
MIT License
177 stars 27 forks source link

Travis CI for sequential exercises #42

Closed AminArria closed 6 years ago

AminArria commented 6 years ago

Issue #16

~Currently only runs tests for the sequential exercises. As more exercises for the other sections are added and existing ones fixed it will run tests over all of them.~ All existing tests run.

NOTE: I don't have sufficient rights to enable Travis in this repository once this is merged.

facundoolano commented 6 years ago

I'll enable it now, you'll probably have to debug it before merging this.

facundoolano commented 6 years ago

done https://travis-ci.org/lambdaclass/erlings

facundoolano commented 6 years ago

You may need to add a commit for it to recognize this.

unbalancedparentheses commented 6 years ago

Please @AminArria add the image that states if things are building fine to the readme. Let me know if you don't know what I am talking about

facundoolano commented 6 years ago

Is there a way of not having Makefiles in all subdirectories? (they all seem to have the same content)

AminArria commented 6 years ago

@unbalancedparentheses added the image

@facundoolano mmm I don't think so, at least not without causing inconvenience to the users. If we remove the Makefiles of each exercise then users won't be able to just do make inside the exercise folder to see if there solution works. There is an alternative, but breaks the repository for Windows users. We set the Makefiles as symlinks.

facundoolano commented 6 years ago

what I was going for is to always run from the root folder

make -> run all the tests make sequential -> run all sequential tests make sequential/bank_accounts -> run that excersice's tests

and maybe one to run a single test

AminArria commented 6 years ago

Mmmm I think it could be done, but syntax would be a bit different because of the two contexts we need to handle, users and travis (our solutions), we could have something like:

make -> run all tests (users)
make test -> run all tests (travis)
make user_sequential -> run sequential tests (users)
make test_sequential -> run sequential tests (travis)
make user_sequential/bank_accounts -> run that exercise's tests (users)
make test_sequential/bank_accounts -> run that exercise's tests (travis)
facundoolano commented 6 years ago

Ok, in that case I would suggest using a target to define what types of tests are being run, and then a parameter to specify folder/module.

make test_solutions -> run all tests (users)
make test -> run all tests (travis)
make test_solutions sequential -> run sequential tests (users)
make test sequential -> run sequential tests (travis)
make test_solutions sequential/bank_accounts -> run that exercise's tests (users)
make test sequential/bank_accounts -> run that exercise's tests (travis)

i.e. one target to run our default tests (make test) and another to run user submited solutions (make test_solutions). (you can change the target names if you find better ones).

AminArria commented 6 years ago

From what I've read (and correct my if I'm wrong) Makefiles don't receive parameters that way, you need to make the variable assignment in the call:

make test_solutions DIR=sequential -> run sequential tests (users)
make test DIR=sequential/bank_accounts -> run that exercise's tests (travis)

A bit annoying. I would prefer for it to be the other way around, targets are directories and a parameter to define if users (default) or travis:

make test -> run all tests (users)
make test PROFILE=travis -> run all tests (travis)
make sequential -> run sequential tests (users)
make sequential PROFILE=travis -> run sequential tests (travis)
make sequential/bank_accounts -> run that exercise's tests (users)
make sequential/bank_accounts PROFILE=travis -> run that exercise's tests (users)
facundoolano commented 6 years ago

Sounds reasonable, as long as you don't manually need to add a new target every time you add a new dir. Can you make it so it assumes that whatever the target is, it's the directory?

unbalancedparentheses commented 6 years ago

The second option looks better in this case, in other cases the first one is better.

AminArria commented 6 years ago

Made the changes for a single root Makefile (only left the ones that do something besides test). It will automatically detect folders and created the targets. No need to touch the Makefile when adding new exercises

facundoolano commented 6 years ago

👍