Skip to content

[WIP] Check Results API #997

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

Closed
wants to merge 54 commits into from
Closed

[WIP] Check Results API #997

wants to merge 54 commits into from

Conversation

awstlaur
Copy link
Collaborator

@awstlaur awstlaur commented Apr 1, 2017

This PR adds a --run-full-report command-line flag whose usage is as follows:

node build/phaseA/pyret.jarr --run-full-report file.arr

This will parse, compile, and run file.arr, and write JSON data to standard out.

No matter what happens in file.arr (and any Pyret dependencies it has), I claim
this API is robust enough to not allow stray messages to be written to stdout.
This is important, because this API will be used by grading scripts to collect
data: a grading script may want to redirect stdout to a file.arr.json file,
and then do its remaining "business logic" outside of Pyret, in the language
of its author's choice.

For examples of input programs and their resulting JSON data, see
TM030 Check Results API

Some notes about the PR and its changeset:

  • This is marked work-in-progress (WIP) because I haven't added tests of the
    proposed functionality. Before I put effort into writing said tests, I would
    like us to agree upon the new interface and expected behavior.

  • The diff on checker.arr is a bit messy: the gist there is that the
    functionallity of results-summary was expanded and moved into the new results-report
    function; now, results-summary depends on results-report.

  • The added toJSON in ffi.js has been robust enough for this use case, but
    I do not claim it can cleanly convert any Pyret data structure into JSON.
    Its cases mirror those in json-structs.arr:tojson, but it does not handle
    string-dicts.

  • load-lib.js has some repeated logic: renderCheckResults and renderCheckReport
    are similar, and renderErrorMessage and renderErrorReport are also similar.
    In theory, the renderCheckResults and renderErrorMessage may be able to depend
    on the render*Report functions, but (I believe) it would require messy stack
    management and thus might not be worth the trouble.

it will not exit with EXIT_ERROR_CHECK_FAILURES when needed.
however, it the main function in pyret.arr will return an
appropriate error code, if we can leverage that

also: this commit fails on  no-diff-standalone
doing this mainly to investigate timeouts on travis ci
each of the 4 phaseX scripts is a symlink to run-phase, which uses
the basename command to get the name of the symlink file and go
from there
(I was going to just use the json lib, but there were annoying
module cycles arising)
Manually resolved conflicts in pyret.arr.
--run-full-report will not work as of this commit
notes:
- maybe-surface-parse has output type
  Either<{exn: ParseError, message: String}, AST>
- surface-parse either returns the ast, or console.error's the
  message and raises the exn
- test-parse.arr was mostly auto-generated from tests/parse/parse.js
This reverts commit c1236f6.

Reverting on this branch to instead opt for merging in the
maybe-surface-parse branch, which has an open PR into horizon
notes:
- make phaseA fails at this commit.
- this should make merging easier
With manual conflict resolution for the following:

Conflicts:
	src/arr/compiler/cli-module-loader.arr
	src/arr/compiler/pyret.arr
	src/js/base/handalone.js
	src/js/trove/load-lib.js
- adds a 'capture-output' field to the options
- render(Check|Error)Report will propagate the captured output
  to the report json
There were technically merge conflicts, but they were very easy to
resolve.
@jpolitz
Copy link
Member

jpolitz commented Apr 8, 2018

@jswrenn I'm curious if this design is useful to you, or if your recent work on doing lots of testing automation supersedes this in useful ways.

@awstlaur
Copy link
Collaborator Author

I'm not coming back to this

@awstlaur awstlaur closed this Mar 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants