search

cs50 / checks

Checks for check50
17 stars 34 forks source link

AP Check50 for Caesar is now testing incorrectly #54

Open curiouskiwi opened 6 years ago

curiouskiwi commented 6 years ago

https://github.com/cs50/checks/blob/master/cs50/2017/ap/caesar/check50

The check uses the /x/check50 but unfortunately, that check follows the current spec requiring a printed "plaintext" and "ciphertext". The AP spec for caesar is based on the 2016 caesar which did not have those print requirements. As such, check50 is failing for students whose code meets the AP specs when running check50 cs50/2017/ap/caesar. Can we update this?

dlloyd09 commented 6 years ago

@Erin-c let's confirm and otherwise just PR the 2017/fall check here?

curiouskiwi commented 6 years ago

@dlloyd09 @Erin-c I think I was somehow misled by the student's problem. The check50 is indeed correctly following the spec. The issue seems to be in the staff solution ~cs50/chapter2/caesar in the IDE, which is using the 2016 version of caesar (without the printed prompts). The student was testing against that, which does not match the check50 test.

The solution should be to fix the staff version.

dlloyd09 commented 6 years ago

@Erin-c easiest fix right now is to probably just change the spec to say that the staff solution lives at ~cs50/pset2/caesar instead of ~cs50/chapter2/caesar, rather than having @kzidane roll out a new IDE release. The APx course ends in a few weeks anyway.

kzidane commented 6 years ago

@dlloyd09 @Erin-c we should probably soft-link these if you want. Just let me know which ones!

curiouskiwi commented 5 years ago

This is still an issue in the 2018 edition. The spec says ~cs50/unit2/caesar for the staff version, which is the one without the prompts. I expect the solution is to change the spec? Or should we update the IDE's version? @dlloyd09 @Erin-c