Skip to content
This repository has been archived by the owner on Sep 25, 2019. It is now read-only.

Bug: Multiple Issues with tests for ES6: Create Strings using Template Literals #135

Closed
johnkennedy9147 opened this issue Jul 12, 2018 · 10 comments · Fixed by #192
Closed
Assignees

Comments

@johnkennedy9147
Copy link
Contributor

johnkennedy9147 commented Jul 12, 2018

Describe the bug
Text for first test should be amended to say array rather than list as this is both what we specify and test for.

Second test only passes if you use double quotes around class attribute

The test for use of template literals has an incorrect regex. The part of the regex testing for </li> has not been properly escaped so / is being treated as end of the regex, with l being treated as a regex flag.

There maybe additional issues with the regex, so I will try and identify all before submitting PR to fix.

To Reproduce
Steps to reproduce the behavior:

  1. Go to create-strings-using-template-literals
  2. Create correct solution
  3. Click Run the Tests
  4. Test fails
    image

Expected behavior
Test should pass

Desktop (please complete the following information):
Reported by multiple campers - not system specific

@scissorsneedfoodtoo
Copy link
Contributor

scissorsneedfoodtoo commented Jul 12, 2018

Hi @johnkennedy9147,

You're right. Even though this challenge's regex test to detect template literals was fixed and improved recently in #90, I think there's still room for further improvement. I just did some testing, and while solutions that use push and map/forEach work fine now, hard coded solutions don't seem to work. Valid template literal strings like <li class="text-warning">${result.failure[0]}</li> would still fail because the test doesn't properly check for the period or the word after.

If you're interested, here's an updated solution that might help for your future PR:

{ 
  "text": "Template strings were used",
  "testString":
    "getUserInput => assert(getUserInput('index').match(/`<li \\s*class\\s*=\\s*(\"\\s*text-warning\\s*\"|'\\s*text-warning\\s*')\\s*>\\s*\\$\\s*\\{(\\s*\\w+\\s*|\\s*\\w+\\.?\\w*?\\s*\\[\\s*[\\w]+\\s*\\]\\s*)\\}\\s*<\\s*\\/li\\s*>`/g), 'Template strings were used');"
}

Would be good to get another pair of eyes on it, though, just to make sure all valid solutions pass. Here's a regexr link that can help. And I'm not the best at regex, so please feel free to change or simplify the pattern as you see fit.

All that said, the second test still needs to be updated to support single quotes like you mentioned. Also, I think I read that you plan on making some edits to the description, which would also be great. Looking forward to your upcoming PR!

@SandySingh
Copy link

@scissorsneedfoodtoo Solution using map is still not working.

@johnkennedy9147
Copy link
Contributor Author

#140 fixed incorrect text in first test

@johnkennedy9147
Copy link
Contributor Author

missing escaping for /li> fixed in #90

@johnkennedy9147
Copy link
Contributor Author

@scissorsneedfoodtoo I was working on the regex and it occurred to me, we have a test that checks the content and then another that checks template literals were used, that being the case in the third test do we really need a complex regex - if we are only testing for template literals then we can just do ^`.*`$

@scissorsneedfoodtoo
Copy link
Contributor

scissorsneedfoodtoo commented Jul 25, 2018

@johnkennedy9147, just saw your PR. Great catch there! In all our work with the third test more flexible we never updated the second to accept single quotes.

About the new regex for the third test, that seems like a great idea. A simple check like that would allow all solutions I can think of to pass, and the current test doesn't have to be so complex since there's another that checks the contents of resultDisplayArray already. Originally I thought the template literals used as examples in the commented out code would cause the test to pass without user input, but thinking about it more that shouldn't be a problem at all.

@johnkennedy9147
Copy link
Contributor Author

@scissorsneedfoodtoo unfortunately the comment is causing the test to pass. the style of test being used looks at the whole content of the code editor, and the style used in test 2 wont work as the strings being tested dont include the quote or backtick to check against. So looks like it will have to be a big horrible regex - think I have a working solution, slightly adapted from your suggestion. Will test it properly tomorrow and raise PR.

@scissorsneedfoodtoo
Copy link
Contributor

@johnkennedy9147, that's too bad. Would be great to simplify here if we could, but it's not a big deal. I was playing with the simplified regex and found something that works, though it relies on a negative lookbehind which isn't supported in Firefox yet.

Looking forward to your PR!

@johnkennedy9147 johnkennedy9147 self-assigned this Jul 28, 2018
@johnkennedy9147
Copy link
Contributor Author

@scissorsneedfoodtoo I was clearly too tired when I looked at this regex the other day
`.*` works unless campers are doing weird stuff. By which I mean it checks the entire input of the editor for a template literal, but there are none in the pre-filled code, even the comments, and there is no reason for someone to put one in the editor except in completing the challenge. If a camper chooses to edit the comments so that they contain template literals thus causing this test to pass when it otherwise wouldn't then so be it. They still need to pass the other two tests on the challenge.

scissorsneedfoodtoo pushed a commit that referenced this issue Jul 31, 2018
replaced the overly long and complex regex which tests for use of template literals with a much
simpler one that has the same effect

ISSUES CLOSED: #135
raisedadead pushed a commit that referenced this issue Jul 31, 2018
## [3.1.2](v3.1.1...v3.1.2) (2018-07-31)

### Bug Fixes

* **challenges:** allow for omitted unit after zero values ([45b573b](45b573b)), closes [#166](#166)
* **challenges:** changed complementary color for blue to orange ([e41f078](e41f078)), closes [#17934](https://github.com/freeCodeCamp/curriculum/issues/17934)
* **challenges:** fix test in hr challenge ([#189](#189)) ([2edb306](2edb306))
* **challenges:** fix third test for template literals ([b8d004e](b8d004e)), closes [#135](#135)
* **challenges:** fixes escaping of ([fd8c9e4](fd8c9e4))
* **scripts:** fix unpack and repack scripts for the new challenge schema ([52ed7cf](52ed7cf))
@raisedadead
Copy link
Member

🎉 This issue has been resolved in version 3.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants