Skip to content

Replace optional key with scenarios #1677

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

Merged
merged 13 commits into from
Oct 6, 2020
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ node_js:
cache: yarn

script:
# TODO: add automation to check the "scenario" field
- bin/check_required_files_present
- bin/check_optional
- yarn test
99 changes: 76 additions & 23 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ Shared metadata for Exercism exercises.

## Contributing Guide

Please see the [contributing guide](https://github.com/exercism/problem-specifications/blob/master/CONTRIBUTING.md)
Please see the [contributing guide](./CONTRIBUTING.md).

## Problem metadata
## Exercise Metadata

Each problem's data lives in a directory under `exercises/`
Each exercise's metadata lives in a directory under `exercises/`.

```text
exercises/
Expand All @@ -26,17 +26,29 @@ exercises/
└── metadata.yml
```

There are three metadata files:
There are three metadata files per exercise:

* `description.md` - the basic problem description
* `metadata.yml` - additional information about the problem, such as where it came from
* `canonical-data.json` - standardized test inputs and outputs that can be used to implement the problem
- `description.md` - the basic problem description
- `metadata.yml` - additional information about the exercise, such as where it came from
- `canonical-data.json` (optional) - standardized test inputs and outputs that can be used to implement the exercise

## Test Data Format (canonical-data.json)
## Test Data (canonical-data.json)

This data can be incorporated into test programs manually or extracted by a
program. The file format is described in [canonical-schema.json](https://github.com/exercism/problem-specifications/blob/master/canonical-schema.json), but it
is easier to understand with an example:
Test data can be incorporated into a track's test suites manually or extracted by a program (a.k.a. a _test generator_).

- Exercises _must_ contain tests that cover the public interface of the exercise (also thought of as "application tests").
- Exercises _may_ contain tests that cover the private or lower-level interface of the exercise (sometimes refered to as "library tests").

- Test cases are immutable, which means that once a test case has been added, it never changes. There are two exceptions:
- The `comments` field _can_ be mutated and thus does not require adding a new test case when changing its value.
- The `scenarios` field _can_ be mutated additively, by adding new scenarios. Existing scenarios must not be changed or removed. Adding new scenarios thus does not require adding a new test case.
- Test cases _must_ all be considered optional, insomuch that a track should determine per test case whether to implement it or not.
- Each test case has a [UUID (v4)](https://en.wikipedia.org/wiki/Universally_unique_identifier) to uniquely identify it.
- If tracks automatically generate test suites from test data, they _must_ do that based on an explicit list of test cases to include/exclude. Test cases must be identified by their UUID, and we'll provide tooling to help keep track of which test cases to include/exclude.

## Test Data Format

The file format is described in [canonical-schema.json](./canonical-schema.json), but it is easier to understand with an example:

```json
{ "exercise": "foobar"
Expand Down Expand Up @@ -117,33 +129,74 @@ is easier to understand with an example:
}
]
}
```

## Scenarios

- The `scenarios` field can use one or more of a predefined set of values, which are defined in a [`SCENARIOS.txt`](./SCENARIOS.txt) file.
- The `scenarios` field can be mutated additively, by adding new scenarios. Existing scenarios must not be changed or removed. Adding new scenarios does therefore does not mean adding a new test case.
- Library tests should have a `library-test` scenario added to allow for easy including/excluding of library tests. Application tests won't have their own scenario, as they must be included and should not be filtered on.

## Changing Tests

As test cases are immutable, a "bug fix" requires adding a new test case. We'll add metadata to test cases to link a re-implementation of a test case to the re-implemented test case.

- Re-implemented test cases _must_ have a `reimplements` field which contains the UUID of the test case that was re-implemented.
- Re-implemented test cases _must_ use the `comments` field to explain why a test case was re-implemented (e.g. "Expected value is changed to 2").
- Track generators _should not_ automatically select the "latest" version of a test case by looking at the "reimplements" hierarchy. We recommend each track to make this a manual action, as the re-implemented test case might actually make less sense for a track.

This is an example of what a re-implementation looks like:

```json
[
{
"uuid": "e46c542b-31fc-4506-bcae-6b62b3268537",
"description": "two times one is two",
"property": "twice",
"input": {
"number": 1
},
"expected": 3
},
{
"uuid": "82d32c2e-07b5-42d9-9b1c-19af72bae860",
"description": "two times one is two",
"comments": ["Expected value is changed to 2"],
"reimplements": "e46c542b-31fc-4506-bcae-6b62b3268537",
"property": "twice",
"input": {
"number": 1
},
"expected": 2
}
]
```

Keep in mind that the description should not simply explain **what** each case
is (that is redundant information) but also **why** each case is there. For
example, what kinds of implementation mistakes might this case help us find?
## Conventions

There are also some conventions that must be followed:

- All keys should follow the [lowerCamelCase](http://wiki.c2.com/?LowerCamelCase) convention.
- If the input is valid but there is no result for the input, the value at `"expected"` should be `null`.
- If an error is expected (because the input is invalid, or any other reason), the value at `"expected"` should be an object containing exactly one property, `"error"`, whose value is a string.
- The string should explain why the error would occur.
- A particular track's implementation of the exercise **need not** necessarily check that the error includes that exact string as the cause, depending on what is idiomatic in the language (it may not be idiomatic to check strings for error messages).
- Test cases that only some tracks should implement, for example because it would unnecessarily increase the complexity of the exercise in some but not all languages, mark it with an `optional`-key. Multiple cases related to the same reason for optionality should have the same key. The decision that a test case is optional will often be made in the PR discussion, so don't worry about it too much while creating a PR.
- Each test case must have a unique UUID specified in its `"uuid"` key. A UUID can be randomly generated using the [UUID Generator](https://www.uuidgenerator.net/version4).
- Descriptions should not simply explain **what** each case is (that is redundant information) but also **why** each case is there. For example, what kinds of implementation mistakes might this case help us find?
- All keys should follow the [lowerCamelCase](http://wiki.c2.com/?LowerCamelCase) convention.
- If the input is valid but there is no result for the input, the value at `"expected"` should be `null`.
- If an error is expected (because the input is invalid, or any other reason), the value at `"expected"` should be an object containing exactly one property, `"error"`, whose value is a string.
- The string should explain why the error would occur.
- A particular track's implementation of the exercise **need not** necessarily check that the error includes that exact string as the cause, depending on what is idiomatic in the language (it may not be idiomatic to check strings for error messages).

## Validation

`canonical.json` files can be validated against its schema using https://www.jsonschemavalidator.net/ with...

The `canonical.json` file can be validated against its schema prior to commiting using https://www.jsonschemavalidator.net/ with...
```
{
"$schema": "https://github.com/exercism/problem-specifications/blob/master/canonical-schema.json"
"$schema": "https://github.com/exercism/problem-specifications/blob/master/canonical-schema.json"
}
```

## New Exercises Require a Glyph

When creating a new exercise the design team needs to be informed so that a new glyph can be created.

- An issue should be opened in [exercism/website-icons](https://github.com/exercism/website-icons/issues) after a PR has been opened in problem-specifications.
- This issue should reference the PR in problem-specifications.

Expand Down
3 changes: 2 additions & 1 deletion OPTIONAL-KEYS.txt → SCENARIOS.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
floating-point
big-integer
floating-point
library-test
unicode
39 changes: 0 additions & 39 deletions bin/check_optional

This file was deleted.

19 changes: 13 additions & 6 deletions canonical-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
, "properties" :
{ "uuid" : { "$ref": "#/definitions/uuid" }
, "description": { "$ref": "#/definitions/description" }
, "optional" : { "$ref": "#/definitions/optional" }
, "scenarios" : { "$ref": "#/definitions/scenarios" }
, "comments" : { "$ref": "#/definitions/comments" }
, "property" : { "$ref": "#/definitions/property" }
, "input" : { "$ref": "#/definitions/input" }
Expand All @@ -95,7 +95,7 @@
, "required" : ["description", "cases"]
, "properties" :
{ "description": { "$ref": "#/definitions/description" }
, "optional" : { "$ref": "#/definitions/optional" }
, "scenarios" : { "$ref": "#/definitions/scenarios" }
Copy link
Member Author

Choose a reason for hiding this comment

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

For now, I've allowed the possiblity of also having scenarios at the test group level.

, "comments" : { "$ref": "#/definitions/comments" }
, "cases" : { "$ref": "#/definitions/testGroup" }
}
Expand All @@ -113,10 +113,17 @@
, "pattern" : "^[a-z0-9]{8}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{12}$"
},

"optional":
{ "description": "An identifier for similar optional test cases (kebab-case)"
, "type" : "string"
, "pattern" : "^[a-z]+(-[a-z]+)*$"
"scenario":
{ "description": "An identifier for a specific scenario (kebab-case)"
, "type": "string"
, "enum": ["big-integer", "floating-point", "library-test", "unicode"]
Copy link
Member Author

Choose a reason for hiding this comment

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

I've taken the liberty of adding the actual values here, which makes the JSON schema validation a lot nicer.

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems easy to miss when adding a new scenario. Could we somehow merge both lists?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought of maybe having a CI check to verify that the two match? The benefit of the file is that is easier to read. This has the benefit of actually verifying the actual scenario values.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we regenerate the schema when the file changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

But a check works too, this isn't blocking

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we regenerate the schema when the file changes?

We could. Good idea.

},

"scenarios":
{ "description": "An array of scenarios that can be used to include/exclude test cases"
, "type" : "array"
, "items" : { "$ref": "#/definitions/scenario" }
, "minItems" : 0
},

"property":
Expand Down