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

Exercise generator #52

Merged
merged 3 commits into from
Nov 17, 2016
Merged

Exercise generator #52

merged 3 commits into from
Nov 17, 2016

Conversation

bmulvihill
Copy link
Contributor

@bmulvihill bmulvihill commented Nov 16, 2016

Overview

Based on #20
Creates an basic exercise generator for the Crystal language track. The generator lives in the src directory. This generator is based on the ruby generator - but is not a complete copy. (Beware I am a Ruby dev, and have been getting into Crystal lately, so I have not completely learned all the Crystal best practices). I have created a sample generator for the hello-world exercise as an example.

Creating a new Generator

  • Navigate to src/generator/exercises and create a new generator file (i.e. hello_world.cr).
  • This file should contain 2 classes
    • An exercise generator which must inherit from the ExerciseGenerator class
    • A test case class which inherits from ExerciseTestCase.
  • The x-common repo must reside at the same level as the developer's crystal directory, and must contain a canonical-data.json file for the given exercise.

Running the Generator

From within the xcrystal directory:

crystal src/generator/generate.cr hello-world

Or build a binary

make build_generator
bin/generate hello-world

Future Work/Issues

This is just an initial implementation and would like to keep this PR fairly focused on just getting a basic generator implemented.

Currently a lot of the test suites write the tested method is a describe block (i.e. describe "#hello"). I am not quite sure a clean way to implement this using the canonical data since it doesn't define the tested methods.

Let me know if you have any questions/concerns/comments!

@bmulvihill bmulvihill closed this Nov 16, 2016
@bmulvihill bmulvihill reopened this Nov 16, 2016
@mhelmetag
Copy link
Contributor

mhelmetag commented Nov 16, 2016

@bmulvihill , I'll look at this after work but I'm glad you reopened it! Looks cool! I would love to add pulling the raw json from x-common's github so we don't have to worry about having x-common locally or it being up to date (I have a branch out that does that so I can extend this after we merge this).

This is awesome!!

Currently a lot of the test suites write the tested method is a describe block (i.e. describe "#hello"). I am not quite sure a clean way to implement this using the canonical data since it doesn't define the tested methods.

I don't think this style of test case is absolutely necessary and the benefit of automated test cases is greater than the burden of not-perfectly-canonical-test-cases.

One thing that I would love to push towards is a unified generator for all exercises but... maybe there's too much variation between them.

@bmulvihill
Copy link
Contributor Author

bmulvihill commented Nov 17, 2016

Thanks for taking a look!

I agree about the x-common problem, I just went with what we were doing on the Ruby repo for now since it is a simple solution.

I also agree a unified generator would be nice, but until the day where all the canonical data adheres to a the same signature/interface creating a simple TestCase class to handle the differences should work.

Copy link
Contributor

@mhelmetag mhelmetag left a comment

Choose a reason for hiding this comment

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

I don't really have much to say about all this 😆 it looks pretty great!

)

def workload
if !name
Copy link
Contributor

@mhelmetag mhelmetag Nov 17, 2016

Choose a reason for hiding this comment

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

I usually like to use unless of if !. Just a nit...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but I have a rebuttal in the case when an else statement is used, I think unless can be confusing, but I could change this to if name and switch the logic around.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yeah. True... I do usually use unless by itself.

Thanks for the update though!

@mhelmetag
Copy link
Contributor

So this is awesome. I think if you're happy with it I'll merge it in and get to working on pulling in the json from github and then knocking out some more exercises. Thanks a bunch again!

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.

None yet

2 participants