Skip to content

binary-search-tree : add test template#2157

Merged
cmccandless merged 8 commits into
exercism:masterfrom
tsamoglou:binary-search-tree-add-test-template
Jan 14, 2020
Merged

binary-search-tree : add test template#2157
cmccandless merged 8 commits into
exercism:masterfrom
tsamoglou:binary-search-tree-add-test-template

Conversation

@tsamoglou

@tsamoglou tsamoglou commented Jan 13, 2020

Copy link
Copy Markdown
Contributor

No description provided.

@tsamoglou tsamoglou requested a review from a team as a code owner January 13, 2020 11:09

@cmccandless cmccandless left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple of things:

  • You seem to be mixing template variables with Python variables. The rendered test code will not have access to variables like case or macros declared in the template (i.e. test_call)
  • I noticed you were having difficulty with subcase groups nested with actual test cases (exercism/problem-specifications#1629). The proper way to deal with this in the template is to check for the cases field in the case object, and handle it accordingly.

With the above in mind, please see my refactored version here.

@cmccandless

Copy link
Copy Markdown
Contributor

Feel free to duplicate my changes if you wish. If you would like to use my commit directly:

  • If you checked "Allow edits from maintainers" when creating this pull request, I can push my commit directly to your branch.
  • If not, I can guide you through pulling my commit from my fork into yours.

@tsamoglou tsamoglou changed the title [WIP] binary-search-tree : add test template binary-search-tree : add test template Jan 13, 2020
…mplate

fix template to generate all tests, refactor for readability
@tsamoglou

Copy link
Copy Markdown
Contributor Author

@cmccandless I did that you say, the branch is ready for commit!

@tsamoglou tsamoglou requested a review from cmccandless January 13, 2020 23:43

@cmccandless cmccandless left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My apologies, my fix had an error that created an extra blank line. I determined I had the necessary permissions and took the liberty of pushing the fix myself.

@cmccandless cmccandless merged commit 89e9994 into exercism:master Jan 14, 2020
tsamoglou added a commit to tsamoglou/python that referenced this pull request Jan 15, 2020
binary-search-tree : add test template (exercism#2157)
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