okpy/ok-client

A Python client for the OK autograding system

ok-client's Issues

Displaying Link Causes Confusion

We should display the backup link when students type in "python3 ok". It should only be displayed when --submit is shown.

Missing value in Scheme test is not reported as an error

If I leave out a value in a scheme test, the test still passes.

For instance, the output radius is missing from this test, but the test still passes:

https://github.com/Cal-CS-61A-Staff/berkeley-cs61a/blob/master/src/proj/scheme/tests/05A.py#L81

Show students what email they've submitted under & change wording for auth

I've seen a lot of students type in their ID Number instead of their CalNet username. Can we change the input prompt to ask for CalNet Username:?

In addition, after submitting - it would be good for students to see what account it's submitted under. Something like

Submission for [email protected] successful 
URL:  http://ok-server.appspot.com/#2323232/submission/123123

If the client doesn't easily have access to this information, we could also send the information down from the server.

Prompt users to authenticate with full berkeley email address

Current authentication looks like this:

Performing authentication
Please enter your CalNet ID.
CalNet ID: example.id

The term "CalNet ID" has been confusing to students in the past, with some students interpreting it to mean their student ID. The prompt should explicitly ask for their Bearfacts email:

Performing authentication
Please enter your Bearfacts email (@berkeley.edu)
Email: [email protected]

Code Review

To release hw1 last night, I pushed a few changes to master. Here are the commits to code review:

0ad34ae
6c9783c
587f854

split() takes no keyword arguments

See https://piazza.com/class/i4q8ow8g8bv7oa?cid=135.

Relevant line of code: https://github.com/Cal-CS-61A-Staff/ok-client/blob/cc7062a30337eff0adb9ff7a3b312aa4841e624f/client/cli/common/assignment.py#L67

I was able to replicate this on my own version of Python 3.2. For some reason, Python 3.2 doesn't seem to allow explicitly naming the maxsplit parameter.

Dump tests only if they've been changed

The best way to do this is to add a modified property method in the Serializable class. Simply having setattr toggle an attribute is probably not enough; consider creating a "snapshot" of the initialization and comparing when dumping tests.

Empty doctest is considered a failed test

https://piazza.com/class/i4q8ow8g8bv7oa?cid=1167

If you define a helper function with no doctest, then we count that as a failure. I'm not sure if there's a good way to distinguish a student-defined helper function from a required function that they removed the doctests for.

Rename file_contents -> BackupProtocol

The name sounds better.

Tests are not guaranteed to be loaded in alphabetical order

glob.glob is defined to return files in an arbitrary order.

Doctest can't parse a line consisting only of ...

See this line for details.

Add a "hidden" field to ok_test suites

Some suites might contain information that students shouldn't see (e.g. a 'setup' that contains reference to a staff solution).

JSON error when .ok config file is malformed

The entire JSON parse error is dumped, uncaught. Exit gracefully with a helpful error message instead.

README should say how to deploy a new version

Some instructions for updating ok would be helpful:

• Where's the version number
• How does setup.py work

Error when running python ok

There's an error when a student runs "py ok". They get this stacktrace. They can't submit either.

Traceback (most recent call last):
File "C:\Python34\lib\runpy.py", line 170, in run_module_as_main
"main", mod_spec)
File "C:\Python34\lib\runpy.py", line 85, in run_code
exec(code, run_globals)
File "ok__main.py", line 9, in
File "ok\client\cli\ok.py", line 122, in main
File "ok\client\cli\common\assignment.py", line 20, in load_config
File "ok\client\sources\common\core.py", line 174, in call
File "ok\client\cli\common\assignment.py", line 51, in post_instantiation
File "ok\client\cli\common\assignment.py", line 86, in load_tests
File "ok\client\sources\doctest__init_.py", line 48, in load
File "ok\client\sources\common\core.py", line 164, in call
File "ok\client\sources\common\core.py", line 186, in setattr
File "ok\client\sources\common\core.py", line 45, in coerce
TypeError

Add more test coverage

Sources and protocols have pretty decent test coverage, but the following modules still need unittests:

• cli/common/assignment.py
• sources/doctest/load_test.py
• sources/ok_test/load_test.py
• utils/network.py

In addition, some integration tests are needed.

Test Unlocking Doesn't save

After unlocking a couple of questions, ok exits and doesn't save the unlocked progress.

Provide a command line utility for generating a default config.json

Something like

ok-publish --new-config

Do we need both protocol.on_start and protocol.on_interact now?

I recall that the original intention of having on_start is to send some info to the server before running on_interact. Now that protocol.on_start and protocol.on_interact are simply run sequentially, one after the other, are the two still necessary? I should note that no protocol currently implements both on_start and on_interact.

Doctest loading should only have to specify the filename, not the individual tests

However, it should be able to have both options (just filename, as well as specific function).

TypeError: int value is not subscriptable

Not sure what the issue is (no screenshots yet) but it seems to be happening to several students. The version that seems to be bundled with the homework is 1.3.8, if that affects anything.

https://piazza.com/class/i4q8ow8g8bv7oa?cid=560

Potential issue with __pycache__

A student fixed an indentation problem in their code, but ok seemed to ignore those changes. Investigate further.

"ordered" field does not work correctly for sqlite test cases with multiple prompts

Determines whether or not OK should check for correct ordering of sqlite output. Default to False.

Evaluate the student's answer, as alternative to exact string matching

This is a continuation of the discussion at Cal-CS-61A-Staff/berkeley-cs61a#135. As an example of the issue, if we expect [1, 2, 3], the unlocking attempt [1,2,3] fails.

@papajohn's proposal:

How about first string compare, then try: repr(eval(g)) except: pass, maybe
inside a timeout, as a second attempt to unlock.

Support doctest non-Python files.

See this link.

Rewrite AnalyticsProtocol

• Record all command line arguments
• Rename to InfoProtocol, since analytics occurs throughout OK, not just in this protocol.

Auth errors with multiple instances of ok

Since we use a single port number (7777), authentication will always error when students have multiple instances of ok running. This might happen when, say, auth is taking too long, so a student opens up a new tab and auths again.

Now, my question is that is this desirable, or should we allow one of the authentications to succeed? (by using a random port number). Also, if students happen to have another service running on 7777, then ok won't interfere, but I think this is much less likely.

In interactive mode (-i flag), Ctrl-D does not quit interactive console.

Not sure if this was a design choice or not.

Authentication not working

https://piazza.com/class/i4q8ow8g8bv7oa?cid=167

Integrate ucb.py with ok

The biggest issue with integration is usage. Ideally, the __init__.py file in OK can define an interface of features (like the main decorator, interact, and trace). However, Python doesn't support direct imports from ZIP-archives (which is what OK is currently distributed as). For example, the following does not work:

from ok import main, interact, trace

The "easiest" way to work is to add ok to sys.path:

import sys
sys.path.insert(0, 'ok')
from ok import main, interact, trace

However, that solution is rather clunky. Thoughts on how to get around this?

GradingProtocol needs to switch output.on() if KeyboardInterrupt occurs

Otherwise, terminal is left with no output.

ok should list all valid tests, perhaps when edit distance is too large

I think we should be more clear what problems ok is able to test. Using Homework 1 as an example, it is not clear that there are no tests for if_function.

Interestingly, running python3 ok -q if_function actually runs the tests for hailstone. I do think that running python3 ok -q hailston and running hailstone is nice, so maybe we should introduce some maximum edit distance we are willing to run a test for. Typing the entire function name out (as opposed to q1) is a bit inconvenient though, so I think somehow mapping q1 to a list of functions to run doctests for would be preferable.

Add Behavioral tests to ok

We should have some kind of visual testing for client-side issues so that any future rewrite doesn't break things. In particular, the SyntaxError wasn't being handled properly until v1.3.3.

Restoring from a backup

Once I've finished #77, I'd like to add a --restore option that would allow students to retrieve a previous version of their assignment that's been backed up to the server. This could be particularly useful for partners on an assignment to share their work with each other.

I should be able to write this, but I'm not sure how I should handle overwriting existing files on the student's computer when replacing them with the backup. Here are the three ways I was thinking of handling this:

Option A: Warn the student that restoring to an older backup will overwrite the files currently on their computer and have them confirm that they want to do this.

Option B: Move the current version of any files that will be replaced into a subdirectory before replacing them.

Option C: Make a new server-side backup before restoring an old backup. This would probably be the most seamless option for students, but it would complicate assignment configurations by requiring that the restore protocol is listed after the backup protocol.

Let me know which if these options you think would work best or if you have some other idea.

RuntimeErrors are assumed to be maximum recursion depth exceeded

It seems to be a result of https://github.com/Cal-CS-61A-Staff/ok-client/blob/6f61f1e0d326fc31b872ec0ad5856131360074f8/client/sources/common/doctest_case.py#L225.

Missing src file doesn't fail.

When running python3 ok without hw01.py in the same directory, we should print some error message and exit. This currently produces no output, which could be confused with all tests passing.

Submit + Update should provide explanation for why the submission failed.

Currently, if the client discovers it needs to update - the submission will fail - attempting to submit a few times before giving up. The submission will succeed on the second try. We should print should a different error message to make this clearer. (Something along the lines of 'ok had to update, please try submitting again')

Backing up your work...
Submitting project... 0% complete
Submitting project... 0% complete
Submitting project... 0% complete
Submitting project... 0% complete
Submitting project... 0% complete
Submission failed. Please check your network connection and try again

Rename config.json

We should rename config.json to be more assignment related. I'm not 100% sure how this'll work, but maybe it can look for all *.json files and then just use the first one that it can parse successfully (and if nothing parses, we can say "no config found").

The reason for this is that it becomes really obvious what ok uses to figure out the assignment- just have "*.json" in there and it'll work.

"Submission Successful" after "Error Loading Assignment"

When there's an assignment load error, the client confusingly prints "submission successful".

Client side confirmation of submission after deadline

Have we thought of adding a warn_date to the assignments .ok file - and if the current time it confirms before sending in a submission? (Similar to glookup's late submission)

A related issue is actually displaying helpful error messages. The server currently can send helpful error messages. Displaying more than just the "Check your network connectivity" that is displayed now on errors would make things clear for students.

Confusing tracebacks

SyntaxErrors (like IndentationErrors) aren't caught and produce an ugly traceback. For example, if hw01.py contains print 1, then the traceback below is produced:

$ python3 ok
=====================================================================
Assignment: Homework 1
OK, version v1.3.2
=====================================================================

Traceback (most recent call last):
  File "/usr/lib/python3.3/runpy.py", line 160, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python3.3/runpy.py", line 73, in _run_code
    exec(code, run_globals)
  File "ok/__main__.py", line 9, in <module>
  File "ok/client/cli/ok.py", line 125, in main
  File "ok/client/cli/common/assignment.py", line 21, in load_config
  File "ok/client/sources/common/core.py", line 174, in __call__
  File "ok/client/cli/common/assignment.py", line 52, in post_instantiation
  File "ok/client/cli/common/assignment.py", line 82, in _load_tests
  File "ok/client/sources/doctest/__init__.py", line 23, in load
  File "ok/client/sources/common/importing.py", line 6, in load_module
  File "/usr/lib/python3.3/importlib/__init__.py", line 90, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1584, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1565, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1532, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 584, in _check_name_wrapper
  File "<frozen importlib._bootstrap>", line 1022, in load_module
  File "<frozen importlib._bootstrap>", line 1003, in load_module
  File "<frozen importlib._bootstrap>", line 560, in module_for_loader_wrapper
  File "<frozen importlib._bootstrap>", line 853, in _load_module
  File "<frozen importlib._bootstrap>", line 980, in get_code
  File "<frozen importlib._bootstrap>", line 313, in _call_with_frames_removed
  File "./hw01.py", line 35
    print 1
          ^
SyntaxError: invalid syntax

Send 1 message to server per ok run

Client is currently sending 2 messages per run, when it should only be 1 message.

Incorrect URL displayed after submission

When I submitted lab 2, the URL displayed in the client (after submission finished) did not contain any file contents.

I believe the client submits multiple times, and it should only show the URL for the submission that actually includes the contents of the files.

OkTest should catch PythonConsoleErrors

If the teardown of a doctest case causes problems, a PythonConsoleException will be raised and unhandled.

Add scheme support

In order to get Scheme support working before the end of March, we need to do the following:

• Add an OkTest suite that supports Scheme tests. OK should hook into a "scheme" or "scheme.py" Python-executable script (most likely situated in the current directory. See 61A's Scheme project for an interface
• Write up a test format for Scheme tests.
• Specific to 61A's Scheme homework assignments: generate *.pyc versions of 61A's completed Scheme project. This won't actually be part of the OK client, but we'll use it for testing.

How can students add tests?

We should have some standard way for students to write their own tests. One obvious option is to have them add a new function in their source file that has doctests, and make an option for ok to run those doctests (even if they aren't specified in the .ok file).

ok gets confused if a test name is a substring of another

For lab 3, running python3 ok -q sum results in

Did you mean one of the following?
    lab03.py:sum_every_other_number
    lab03.py:sum
Error loading assignment

ExportProtocol mistakenly reports "Download failed" on second attempt

@jathak I tried using the --export command as an admin. I was able to download the submissions to a submissions directory. On a second invocation, however, it reports that the Download failed. I'm not sure why this error is occurring.

Add --backup option for reliable backup

This is functionally equivalent to --submit, except that the backup will not be marked as "Submit". I can implement this once #53 is merged.

--submit cannot connect to internet

Here's what I get when I try to run python3 ok --submit
Backing up your work...
Submitting project... 0% complete
Submitting project... 0% complete
Submitting project... 0% complete
Submitting project... 0% complete
Submitting project... 0% complete
Submission failed. Please check your network connection and try again

However, I can still browse other web pages from my computer. All other ok commands seem to work as well (I was able to authenticate).

My computer runs on OSX Yosemite.