exercism / csharp

Exercism exercises in C#.
https://exercism.org/tracks/csharp
MIT License
344 stars 346 forks source link

Add Triangle Test Generator #423

Closed jpreese closed 6 years ago

jpreese commented 7 years ago

This issue is part of an overall initiative to complete #195.

The documentation on how to add generators can be found here. If you get stuck or have any questions about adding this generator, please do not hesitate to reach out!

bmeverett commented 6 years ago

I can take this one. Should the Generators.sln compile as soon as I add Exercises/Triangle.cs? Or will it only compile after I override UpdateConcicalData, although I set a break point and it doesn't even seem to be getting to the class but crashes when parsing the JSON.

jpreese commented 6 years ago

Thanks for taking a look @bmeverett! The project should build as soon as you add Triangle.cs to the project. Make sure that you're inheriting from the Exercise base class.

bmeverett commented 6 years ago

@jpreese sorry about that, it compiles but crashes when trying to generate the Triangle tests.

Sent with GitHawk

jpreese commented 6 years ago

Thats no good. Can you provide some more information about the error that you're getting?

bmeverett commented 6 years ago

Here is the stack trace of the error I'm getting

System.Collections.Generic.KeyNotFoundException: The given key was not present in the dictionary.
   at System.ThrowHelper.ThrowKeyNotFoundException()
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Generators.Input.CanonicalDataCasesJson.IsEmptyJArray(CanonicalDataCase x, IGrouping`2 groupedProperties) in C:\Users\beverett\Documents\Github\csha
rp\generators\Input\CanonicalDataCasesJson.cs:line 56
   at Generators.Input.CanonicalDataCasesJson.<>c__DisplayClass4_0.<GetEmptyJArrays>b__0(CanonicalDataCase canonicalDataCase) in C:\Users\beverett\Documen
ts\Github\csharp\generators\Input\CanonicalDataCasesJson.cs:line 53
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at Generators.Input.CanonicalDataCasesJson.ConvertEmptyJArrayToArray(CanonicalDataCase[] canonicalDataCases) in C:\Users\beverett\Documents\Github\csha
rp\generators\Input\CanonicalDataCasesJson.cs:line 32
   at Generators.Input.CanonicalDataCasesJson.ToArray(JToken casesToken) in C:\Users\beverett\Documents\Github\csharp\generators\Input\CanonicalDataCasesJ
son.cs:line 17
   at Generators.Input.CanonicalDataCasesJsonConverter.ReadJson(JsonReader reader, Type objectType, Object existingValue, JsonSerializer serializer) in C:
\Users\beverett\Documents\Github\csharp\generators\Input\CanonicalDataCasesJsonConverter.cs:line 13
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.DeserializeConvertable(JsonConverter converter, JsonReader reader, Type objectType, Objec
t existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerCon
tract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProp
erty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty memb
er, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)
   at Generators.Input.CanonicalDataParser.Parse(Exercise exercise) in C:\Users\beverett\Documents\Github\csharp\generators\Input\CanonicalDataParser.cs:l
ine 19
   at Generators.Program.RegenerateTestClasses(Options options) in C:\Users\beverett\Documents\Github\csharp\generators\Program.cs:line 44
   at CommandLine.ParserResultExtensions.WithParsed[T](ParserResult`1 result, Action`1 action)
   at Generators.Program.Main(String[] args) in C:\Users\beverett\Documents\Github\csharp\generators\Program.cs:line 18
jpreese commented 6 years ago

So this is the code I have for Traingle.cs (added it to Generators/Exercises/Triangle.cs), I didn't change anything else.

using System;
using System.Collections.Generic;
using System.Text;

namespace Generators.Exercises
{
    public class Triangle : Exercise
    {
    }
}

If this matches what you have and still errors, could you push the branch that gives you this error? I'll be able to check it out and debug locally.

bmeverett commented 6 years ago

Pushed to https://github.com/bmeverett/csharp/tree/423-triangle-generator and I copied and pasted what you had and it still errors

jpreese commented 6 years ago

Awesome thanks! It looks like your branch is a little behind. We made some changes to the generator framework recently, so you'll want to pull those in.

I checked out your branch and was able to reproduce the issue. After merging in master, everything worked.

bmeverett commented 6 years ago

Thank you! That did it!

bmeverett commented 6 years ago

So I think the triangle data is throwing me off.

Property- equilateral this isn't a property of the class, I get that it's supposed to be TriangleKind.Equilateral sides- the sides is an array, do we need to break this up or change the class to input an array as a parameter expected value - true/false should we return the enum value here?

I guess I would think the property should be Kind and then the expected value should be one of the eunm values? Let me know if this makes sense or I'm totally missing something.

ErikSchierboom commented 6 years ago

@bmeverett Great observation. What I think we should do is to modify our stub and example implementation to drop the enum and simplify add three boolean tests: scalene, isosceles and equilateral (and perhaps add a more idiomatic IsScalene, IsIsosceles and IsEquilateral). In most cases it makes sense to just follow the canonical data, and I think mapping this canonical data to a format where an enum is used will actually be quite hard.

By the way, as the canonical data descriptions are overlapping, you'll need to use the full description path as the test method name (to make the method names unique). This is possible since the merge of this commit, so I would recommend rebasing to the latest master if that commit is not in your history. Once it is, you can set the UseFullDescriptionPath to true to use the full path (see this line for an example).