This issue will serve as a way to track all the work needed to remove the unnecessary assert messages from the yaml testString. For all challenges which only have a text
for each testString
assert (most of the front-end challenges), we can simply remove the message argument, because the text
is used for displaying both in the tests frame and in the fake console.
Any PRs created for this issue will only affect English curriculum files and only change two things:
text
with assert message argument value if the assert message gives a better test message and remove the assert message argument from the testString
.text and the assert message argument values match, just remove the assert message argument from the
testString`.I have created a script which should capture all of the applicable tests. I will create PRs for only the following sections which are known to not have multiple assert messages in the testString
per single text
tests. I will limit the number of files per PR to make review easier on GitHub.
At the time of this issue's creation, there is only one open PR https://github.com/freeCodeCamp/freeCodeCamp/pull/35770 (waiting on final approval before being merged), which will need to have it's testString assert message arguments reviewed/removed.
Hey @RandellDawson I was just thinking about how best to review these. The problem being that, with so many, human error is likely to creep in.
As such, would it be possible to group them so that all the challenges with solutions are in one lot of PRs and all those without are in another? My thinking was just that we could focus our attention on the ones that're not being tested and the ones that are wouldn't need as much scrutiny.
What do you reckon?
Let me see how many challenges which are in these PR does actually do not have solutions (probably a lot of them in the Coding Interview Prep section). If I update each PR body with a list of PRs without solutions, would that be enough?
@thecodingaviator I went back through and either made a new commit on an existing PR or created a new PR to remove the assert message arguments from the testString
which either were surrounded by double quotes or two single quotes. I think this should take care of 99.99% of the arguments in the curriculum sections which this issue aimed to deal with.
@RandellDawson I think we merge all the PRs in that case because looking for 0.01% of the errors would be like finding a needle in a haystack.
cc: @moT01 @ojeytonwilliams @Manish-Giri
I think the only ones worth looking at all are the ones without solutions. If they have solutions npm run test
will complain if the syntax is wrong or they suddenly start giving the wrong answers.
I think we merge all the PRs in that case because looking for 0.01% of the errors would be like finding a needle in a haystack.
It is not that they are errors. It is just that they would not be removed (which does not hurt anything).
All PRs related to this issue have been merged now. It's up to us to decide whether to close the issue and keep removing message as we keep coming across them or let it be open.
In one of the PRs in this series, @ojeytonwilliams noticed that during one of my manual removals (not script based) of an assert message argument, I accidentally deleted the 2nd argument of the assert.equals statement. It was caught by Travis and fixed. This got us thinking about the fact that not all challenges have solutions and this series of PRs contained affected 181 challenges which do not have solutions and also have assert.equals variants,
I have compiled a list of all 181 challenges and the testStrings which contain an assert.equals variant. Everyone should review this list of assert statements and if you find any where the 2nd argument was removed by mistake, a PR should be created to fix them.
@thecodingaviator I am not making any further mass update PRs which remove any more of the assert message arguments. At this point, I think we will remove them if we come across any while making changes to challenges on future PRs. The main mission (reducing the amount of code in the repo) has been accomplished.
This issue can be closed once a few mods have responded back if any assert.equals variants are missing the 2nd argument.
I had a look at the gist and only found one thing: https://github.com/freeCodeCamp/freeCodeCamp/blob/master/curriculum/challenges/english/08-coding-interview-prep/rosetta-code/gamma-function.english.md still has assert messages. Everything else looked fine, though I can't be 100% sure I haven't missed something.
@ojeytonwilliams Feel free to create a PR to remove the assert messages from that one challenge.
@ojeytonwilliams Are you ok with me closing this issue now?
@RandellDawson Sure, I think we're in a good enough state now.
Most helpful comment
I think the only ones worth looking at all are the ones without solutions. If they have solutions
npm run test
will complain if the syntax is wrong or they suddenly start giving the wrong answers.