Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP Check50 for Caesar is now testing incorrectly #54

Open
curiouskiwi opened this issue Apr 1, 2018 · 5 comments
Open

AP Check50 for Caesar is now testing incorrectly #54

curiouskiwi opened this issue Apr 1, 2018 · 5 comments
Assignees
Labels

Comments

@curiouskiwi
Copy link
Contributor

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?

@dmalan dmalan added the bug label Apr 1, 2018
@dlloyd09
Copy link
Contributor

dlloyd09 commented Apr 2, 2018

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

@curiouskiwi
Copy link
Contributor Author

@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
Copy link
Contributor

dlloyd09 commented Apr 5, 2018

@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
Copy link
Member

kzidane commented Apr 5, 2018

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

@curiouskiwi
Copy link
Contributor Author

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

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

No branches or pull requests

5 participants