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

Test generator v2 #264

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

Test generator v2 #264

wants to merge 3 commits into from

Conversation

Nezteb
Copy link
Contributor

@Nezteb Nezteb commented Sep 18, 2023

There was a previous PR for this: #72

It was very stale and I accidentally messed up its git history while trying to fix conflicts and such.

I figured it'd be simpler/easier to just pull David's entire test generator file, tweak it to account for the new project file structure, and then push it to this new branch. I added his name/username to the file to replace the lost attribution. 😭

@Nezteb Nezteb requested review from BNAndras and a team September 18, 2023 19:39
@Nezteb
Copy link
Contributor Author

Nezteb commented Sep 18, 2023

TODO: After this merges, we should make a followup PR that actually runs the formatter. I'd do it in this one, but it would massively bloat this PR and distract from the actual changes. 😅

You can also enter the `scripts` directly and use either of the following directly:

```
haxe testgen.hxml acronym
Copy link
Member

Choose a reason for hiding this comment

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

testgen.hxml appears to be in the root folder, not scripts


**Not every case has been tested so there may be cases where a test is improperly generated.**

config.json template:
Copy link
Member

Choose a reason for hiding this comment

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

{
"slug": "exercise-slug",
"name": "exercise name",
"uuid": "uuid" , <-- from ./bin/configlet uuid
"practices": [
... array of strings ...
],
"prerequisites": [
... array of strings ...
],
"difficulty": number between 1 - 10
},


public static function main()
{
// TODO: Configlet is able to create the directories/files we need:
Copy link
Member

Choose a reason for hiding this comment

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

If we do run configlet to build the structure, the new exercise already needs to be listed in the track config.json within the practice array under exercises. If it isn't, configlet won't add the files so we should alert the user and bail.

var exercisesDir = "../exercises/practice/";
var dir = '${exercisesDir}${exercise}/';

// create exercise folder
Copy link
Member

Choose a reason for hiding this comment

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

Running configlet sync for the exercise will create the correct files for us so this portion where we create the missing folders / files isn't needed

FileSystem.createDirectory('${dir}test');
}

// create string for tests.toml; first line is always [canonical-tests]
Copy link
Member

Choose a reason for hiding this comment

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

configlet sync will give us a generated test file. It'll consist of about ten one-line comments, a newline, and then alternating test GUIDs followed by a description. There might be a few additional lines of text but each test ends with a newline before the next one.

Here are some examples:

[8b2c43ac-7257-43f9-b552-7631a91988af]
description = "foo"

[180a8ff9-5b94-43fc-9db1-d46b4a8c93b6]
description = "bar"
include = false
comment = "optional comment"

[33eb6f87-0498-4ccf-9573-7f8c3ce92b7b]
description = "baz"
include = false

[c125dab7-2a53-492f-a99a-56ad511940d8]
description = "foobar1"
include = false

[a0c7b9b8-0e89-47f8-8b4a-c50f885e79d1]
description = "foobar2"
include = false
reimplements = "c125dab7-2a53-492f-a99a-56ad511940d8"

[d7982c4f-1602-49f6-a651-620f2614243a]
description = "foobar3"
reimplements = "a0c7b9b8-0e89-47f8-8b4a-c50f885e79d1"

I'd expect foo and foobar3 to be added but the others to be skipped.

testCases.set(story, new Array<String>());
}

// add description to .meta/test.toml
Copy link
Member

Choose a reason for hiding this comment

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

There's no need to edit tests.toml. It's an auto-generated file so any manual edits will be lost anyways.

var testCriteria = '${toUpperCamel(exercise)}.${method}(${inputs.join(", ")}).should.${shouldAssert}(${expected.obj});';
if (testCases.get(story).length != 0)
{
testCriteria = 'pending("Skipping");\n\t\t\t\t' + testCriteria;
Copy link
Member

Choose a reason for hiding this comment

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

We don't use pending("Skipping") but xit instead of it to mark a test as pending. Every test after the first one globally should be pending even if it's in a different describe block

Sys.println("Don't forget to update config.json!");
}

private static function generateTests(cases:Array<Dynamic>, methods:Map<String, {order:Int, method:String}>, stories:Array<String>,
Copy link
Member

Choose a reason for hiding this comment

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

Make sure the tests and any describe sections are contained within an outer describe section. That outer describe should be set to the exercise name. That nested structure will make the test results easier to read for students, and it'll also break up the CI output better.

}

// create String array for README; add exercise as title
var readme = new Array<String>();
Copy link
Member

Choose a reason for hiding this comment

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

configlet will give us the proper docs synced from problem-specifications so we won't need this section

File.copy('${exercisesDir}hello-world/test.hxml', '${dir}test.hxml');

Sys.println("Generation Successful");
Sys.println("Don't forget to update config.json!");
Copy link
Member

Choose a reason for hiding this comment

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

Which config.json does this refer to? It'd be a good idea to tell people to update the exercise-specific config.json to add themselves as an author. The track-wide config.json in the root folder of the repo should have already been updated before running the script so there's no need to remind them to update that one.

```


Workflow:
Copy link
Member

Choose a reason for hiding this comment

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

The first step should be to add the exercise to the track's config.json. That allows us to run configlet sync inside this script to set things up. Then they should proof the tests before updating Example.hx. They should update the exercise's config.json to add themselves as an author, and then make the PR.

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