Skip to content

Update README.md #1747

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Update README.md #1747

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ Test data can be incorporated into a track's test suites manually or extracted b
- 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:
- Test cases are immutable, which means that once a test case has been added, it never changes. There are a couple of exceptions:
- The `description` field _can_ be mutated and thus does not require adding a new test case when changing its value.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- The `description` field _can_ be mutated and thus does not require adding a new test case when changing its value.
- The `description` field _can_ be mutated to fix clarify the test or fix typos, in which case it does not require adding a new test case when changing its value. If the material meaning of the description changes, a new test is required.

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'd rather not add this, as I feel it is vague enough to potentially lead to interpretation issues. There is also not much risk involved in renaming a description for the following reasons:

  1. Each PR requires three people to approve before it can be merged. This significantly decreases the chance of weird things being merged.
  2. Tracks can now use the uuid property to identify a test case. In fact, allowing the description to be changed might discourage tracks from doing so :)

So I'd like to merge this as is. If it turns out this leads to any problematic situations, we can always make the rule more strict.

Copy link
Member

Choose a reason for hiding this comment

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

I think #1746 vs #1744 are good examples and would be caught by at least one person, I think.

- The `comments` field _can_ be mutated and thus does not require adding a new test case when changing its value.
- The `description` 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.
Expand Down