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

Generate test files #312

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Conversation

OceanOak
Copy link
Contributor

@OceanOak OceanOak commented Oct 23, 2022

First attempt at solving issue #309.

  • Created a set of yaml files, one for each function (Int module only).
  • Generated some test files from the JSON version of the YAML files in Rescript, Ocaml, and FSharp.

Issues:

  • Adding labels to Ocaml functions.

Maybe something like this can solve this issue:

        "inputs": [
            {"label": "", "value": 3},
            {"label": "by", "value": 2}
         ],
  • Expecto.expect takes a message parameter at the end to print if the test fails. Should we make it generic or is there a way to customize it to each test?
  • "module" is a reserved keyword in Rescript. Is it possible to rename the module field in the YAML file so that we can use it, or is there another way around this?

Copy link
Member

@pbiggar pbiggar left a comment

Choose a reason for hiding this comment

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

This looks great!

Can you generate all the int tests into one file (IntTests.res, etc) per language?

To get the module name, see here https://rescript-lang.org/docs/manual/latest/use-illegal-identifier-names

Note that the convention in unit tests is to put the actual argument as the first parameter. I don't know if that's the case for all the test suites we're using, best to check to confirm.

For labelled arguments, I'd add a label to each definition in the yaml. Use null if there isn't a label.

@@ -0,0 +1,45 @@
{
"module": "Int",
Copy link
Member

Choose a reason for hiding this comment

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

Let's not check in the json files

@OceanOak
Copy link
Contributor Author

OceanOak commented Oct 23, 2022

Can you generate all the int tests into one file (IntTests.res, etc) per language?

Generating All the int tests into one file required embedding raw JavaScript (in order to use appendFileSync).

Note that the convention in unit tests is to put the actual argument as the first parameter. I don't know if that's the case for all the test suites we're using, best to check to confirm.

I am not sure if I understood this correctly. Consider the following example:
test "clamp(3,7,1)" (fun () -> expect (fun () -> Int.clamp 3 7 1) |> toThrow);
Did you mean that the number comes first, then the lower and upper bounds? if not could you please give an example?

For labelled arguments, I'd add a label to each definition in the yaml. Use null if there isn't a label.

Which definition? I've tried different ways to do it. However, because we are using Array.joinWith, I believe it must be in the same location as the input's value in order to obtain the correct label for each input.

I was testing the remaining files and I discoverd a new issue.
image
In the previously tested files, all of the inputs were of the type int. When I tried to generate tests for the fromString function, I noticed that the quotes were removed. I'm not sure what's causing this problem or if the same code can be used for both cases.

@pbiggar
Copy link
Member

pbiggar commented Oct 24, 2022

Can you generate all the int tests into one file (IntTests.res, etc) per language?

Generating All the int tests into one file required embedding raw JavaScript (in order to use appendFileSync).

You can avoid this by gathering up the strings and writing the whole file out at once.

Note that the convention in unit tests is to put the actual argument as the first parameter. I don't know if that's the case for all the test suites we're using, best to check to confirm.

I am not sure if I understood this correctly. Consider the following example: test "clamp(3,7,1)" (fun () -> expect (fun () -> Int.clamp 3 7 1) |> toThrow); Did you mean that the number comes first, then the lower and upper bounds? if not could you please give an example?

That one is fine. It's for things that go a bit like this:

let actual = Int.add(4,5)
let expected = 9
Expect.toEqual actual expected

Usually the error message when these tests fail will refer to the first one as "actual" and the second one as "expected".

For labelled arguments, I'd add a label to each definition in the yaml. Use null if there isn't a label.
Which definition? I've tried different ways to do it. However, because we are using Array.joinWith, I believe it must be in the same location as the input's value in order to obtain the correct label for each input.

Ah, I see the confusion. I meant the parameter definition should specify if it should be labelled:

parameters:
  - name: a
    type: int
    label: false # like this

I was testing the remaining files and I discoverd a new issue. image In the previously tested files, all of the inputs were of the type int. When I tried to generate tests for the fromString function, I noticed that the quotes were removed. I'm not sure what's causing this problem or if the same code can be used for both cases.

You should be able to dynamically find out the type of the contents here, whether they're ints or strings or lists, etc - if they're strings they should be printed with quotes.

@OceanOak
Copy link
Contributor Author

OceanOak commented Oct 24, 2022

You can avoid this by gathering up the strings and writing the whole file out at once.

I don't think that would be possible. The tests are generated one at a time because the functions are in separate files. Please correct me if I'm wrong.

Ah, I see the confusion. I meant the parameter definition should specify if it should be labelled:

parameters:
  - name: a
    type: int
    label: false # like this

And where will we add the labels?
Should the parameters always be referred to as a,b,c,etc. or should the name match the label?
Example:

parameters:
  - name: a
    type: int
  - name: by
    type: int

You should be able to dynamically find out the type of the contents here, whether they're ints or strings or lists, etc - if they're strings they should be printed with quotes.

I think the issue stems from the types into which we are parsing the data. Here the inputs are always arrays of ints, and the output is an int. is there a way to make accept different types (ints, floats, strings, arrays, etc.)? I tried Js.Dict.t, but it didn't work.

type tests = {
  inputs: array<int>,
  output: int,
}

type data = {
  \"module": string,
  name: string,
  parameters:array<parameter>,
  tests: array<tests>,
}

@scope("JSON") @val
external parseIntoMyData: string => data = "parse"

@pbiggar
Copy link
Member

pbiggar commented Oct 24, 2022

You can avoid this by gathering up the strings and writing the whole file out at once.

I don't think that would be possible. The tests are generated one at a time because the functions are in separate files. Please correct me if I'm wrong.

Ah, I think you're saying that you run the script on each yaml? In that case, you'd run the script on all the yamls in a directory and generate it all at once.

Ah, I see the confusion. I meant the parameter definition should specify if it should be labelled:

parameters:
  - name: a
    type: int
    label: false # like this

And where will we add the labels? Should the parameters always be referred to as a,b,c,etc. or should the name match the label? Example:

parameters:
  - name: a
    type: int
  - name: by
    type: int

I meant the label field would indicate if this parameter should be a label in languages that support it. So it would be a boolean field, and the parameter would have the name in name.

You should be able to dynamically find out the type of the contents here, whether they're ints or strings or lists, etc - if they're strings they should be printed with quotes.

I think the issue stems from the types into which we are parsing the data. Here the inputs are always arrays of ints, and the output is an int. is there a way to make accept different types (ints, floats, strings, arrays, etc.)? I tried Js.Dict.t, but it didn't work.

type tests = {
  inputs: array<int>,
  output: int,
}

type data = {
  \"module": string,
  name: string,
  parameters:array<parameter>,
  tests: array<tests>,
}

@scope("JSON") @val
external parseIntoMyData: string => data = "parse"

Hmmm, maybe inputs would be a Json.t (https://rescript-lang.org/docs/manual/latest/api/js/json#t)?

@OceanOak
Copy link
Contributor Author

OceanOak commented Oct 24, 2022

I tried a completely new method to generate the test files.
It works successfully in the following cases:

  • Functions with one input (int)
  • Functions with multiple inputs (ints)
  • Functions with one input (string)
  • Functions with one input (char)
  • Functions with one or multiple inputs (bool)

Tried to use it on Array.length function and I'm facing this issue
I get
test ("length([a,b])", () => expect(length([a,b])) |> toEqual(Eq.int, 2))
instead of
test ("length(["a","b"])", () => expect(length(["a","b"])) |> toEqual(Eq.int, 2))
I haven't yet tried it with different types of inputs (lists, tuples,etc.) to add the missing edge cases (I have to create the yaml files first).
I believe the code could be optimized but for now I'm just trying to get it to work with any type of input.

@OceanOak
Copy link
Contributor Author

Ah, I think you're saying that you run the script on each yaml? In that case, you'd run the script on all the yamls in a directory and generate it all at once.

Yes, I am running the script on each yaml (json). As a result, new files are generated (one for each language) containing the tests for a specific function.

@OceanOak
Copy link
Contributor Author

OceanOak commented Nov 1, 2022

While Converting the FloatTest's YAML files to JSON  I've noticed that 5. and 5.0 are converted to 5
Tried to escape the "." by using:

    input: >-
      5.

This worked fine for functions that have one input. However, for functions with multiple inputs I got the following result
"inputs": "[3., 7., 1.] "
It is no longer considered as a function with three inputs.
Any thoughts on how to solve this issue? ([#Zero, 1.2] is another example where it would not work)

Edit: To get around this issue, I had to wrap inputs that needed to be escaped in double quotes which worked, but I'm not sure if it's a good idea.

Other issues:

  1. FloatTest round function Ocaml version: (`Zero ->#Zero)

Original test:

  test "`Zero" (fun () ->
              expect (round ~direction:`Zero 1.5) |> toEqual Eq.float 1. ) ;

Generated test:

test "round(~direction=#Zero,1.5)" (fun () ->
              expect (Float.round ~direction:#Zero 1.5) |> toEqual Eq.float 1.) ; 
  1. Because I am getting Eq."type" from the function's return type, I'm getting Eq.radians instead of Eq.float.

Example:

test ("atan(1. /. 1.)", () => expect(Float.atan(1. /. 1.)) |> toEqual(Eq.radians, 0.7853981633974483)) 
test("1 / 1", () => expect(atan(1. /. 1.)) |> toEqual(Eq.float, 0.7853981633974483))
  1. Same issue with OptionTest and TupleTest

image

I also think that we need a better naming system for the tests. We are now naming them based on the function name plus its inputs, which every once in a while causes problems. Here is an example:
image

@OceanOak OceanOak requested a review from pbiggar November 2, 2022 20:43
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.

2 participants