Freecodecamp: "Escaping quotes in literal strings" test needs to be updated

Created on 27 Dec 2017  路  19Comments  路  Source: freeCodeCamp/freeCodeCamp

Challenge Name

Beta: Escaping quotes in literal strings

Issue Description

Currently, the first test expects 6 escaped quotes and 10 normal quotes. The easiest way to pass it right now would be this:

var myStr = "I am a \"double quoted\" string inside \"double quotes\"." ;
// \"\" " " " " " " " "

馃槀
This test was made to expect this many quotes as a workaround for an issue that no longer exists.

What should be changed

This test should only expect 4 normal quotes and 2 escaped quotes. You can find the test here:
https://github.com/freeCodeCamp/freeCodeCamp/blob/5ee49d6f88217abcc0403a4d87b617d3b39375aa/seed/challenges/02-javascript-algorithms-and-data-structures/basic-javascript.json#L1058

Check out CONTRIBUTING.md to get your local development environment set up.

If you need help, please go to our contributors chatroom. We're always happy to answer any questions!

Happy coding!


This issue was brought to my attention by @ncaron on Gitter. Thank you Niko!

Related to #16329

first timers only help wanted

Most helpful comment

@systimotic sure thing! I can tackle both issues in a single PR!

All 19 comments

@systimotic this is an excellent example of how we should write up issues for first timers. Thanks.

/cc @freeCodeCamp/moderators IMHO, we should follow this practise when we can and enable more contributions.

Hi! Could I give this a stab? I have been looking for a first-timer friendly issue. I have my local project set up and ready to go.

@raisedadead Thanks 鉂わ笍

@meda123 For sure!

@systimotic @meda123
Changing the first part of the test to this:
"assert(code.match(/\\\\\"/g).length === 4 && code.match(/[^\\\\]\"/g).length === 2
seems to work but I think it's pretty weird because if you look at the master branch the test is the same as the staging branch with the difference that it passes.

@ncaron A lot of changes have been made since then, so it's difficult to compare the two 馃檪 The main freecodecamp.org runs on backup/master, which also has 4 and 2 instead of 6 and 10

At one point, 6 and 10 were the numbers we had to use to make this run correctly:

Previously, the tail code from the seed was included in the code variable used in the tests. This was a new change in the beta that previously broke this test. This has been reverted, the tail is no longer included in the code. Because of that, the test can also be reverted to what it was before.

The extra quotes it's looking for are the quotes in the tail. When the tail was included in the code, the test needed to account for the quotes in the tail code. Now that the tail is no longer included in the code, the tests no longer need to account for the quotes in the tail, so the numbers can be reverted back to 4 and 2.

All in all, the original instructions should still work fine 馃憤

@meda123 I was just made aware of #16332 by @mstellaluna which notes that the next challenge has the same issue. Can you tackle both in a single PR? If not, that's cool too, let me know and I'll add more information to the other issue so somebody else can pick it up.
https://github.com/freeCodeCamp/freeCodeCamp/blob/5ee49d6f88217abcc0403a4d87b617d3b39375aa/seed/challenges/02-javascript-algorithms-and-data-structures/basic-javascript.json#L1109
Should be 4 and 2

@systimotic sure thing! I can tackle both issues in a single PR!

Hi @meda123! How's your progress on this?

@systimotic So sorry for the delay! was bogged down at work. I can tackle this in the coming week, but if there is someone else who can do it faster I don't want to slow the team down.

Hello @systimotic ! I would like to take this if it's ok.

Hi @adityajoshi sure thing, happy contributing!

Hi @adityajoshi! Do you still plan on working on this issue? If not, I'd love to work on it!
@systimotic @raisedadead Do we need to be assigned the issue before we begin working on it?

Thanks!

Do we need to be assigned the issue before we begin working on it?

Unless there is an open PR, any issue tagged as such is open for contributions.

Hello @systimotic , @raisedadead
I did change for the first issue as mentioned above.
Need guidance for the second issue. Test fails for it.

@adityajoshi The PR merged is not on the parent FCC staging but on your local forked copy, or else merging would have changed the code in the FCC staging (I cross-checked it and it's not).

@raisedadead We might need to add one more instruction to Contributing.md to NOT change the head repo to local forked copy instead of the parent FCC repo, as this might confuse first timers that they merged their commits (_although they can't merge their own commits on the parent repo_) but the changes have still not reflected.

If you suggest, i will open an issue regarding this.
#16477

@saurishkar understood.
Still I am facing difficulty for the second change. Did change as per suggested above. ( ie. made 4 and 2 from 6 and 4 )
I am guessing I have to make "solutions" tag empty as per prior code.
Please correct me if I am wrong.

If you suggest, i will open an issue regarding this.

@saurishkar yes, please.

@adityajoshi if you are adding updating tests that break the currently provided solution in challenge, you should also change the solution to pass the tests.

For fixing the head if accidental commits are made to the staging branch of ones own fork via pull-request, etc.:

On your local machine

git checkout staging
git fetch upstream staging
git reset --hard upstream/staging
git push --force origin staging

where your remotes [git remote -v] are:

origin  https://github.com/USERNAME/freeCodeCamp.git (fetch)
origin  https://github.com/USERNAME/freeCodeCamp.git (push)
upstream    https://github.com/freeCodeCamp/freeCodeCamp.git (fetch)
upstream    https://github.com/freeCodeCamp/freeCodeCamp.git (push)

Hello,

I am not sure if this is fixed. I am running Chromium - Version 66.0.3359.181 (Official Build) Built on Ubuntu , running on Ubuntu 16.04 (64-bit)

image

If I am mistaken, please let me know.

Thank you

Was this page helpful?
0 / 5 - 0 ratings

Related issues

no-stack-dub-sack picture no-stack-dub-sack  路  44Comments

QuincyLarson picture QuincyLarson  路  40Comments

ryanarnouk picture ryanarnouk  路  39Comments

pbnj picture pbnj  路  45Comments

povilasraciunas picture povilasraciunas  路  59Comments