PreTeXtBook / pretext

PreTeXt: an authoring and publishing system for scholarly documents
https://pretextbook.org
Other
254 stars 203 forks source link

Validate WeBWorK XML output #980

Closed Alex-Jordan closed 5 years ago

Alex-Jordan commented 5 years ago

This does some simple testing on the returned content from each webwork problem. It's highly likely my Python is crude and you will see improvements.

Take the minimal webwork example and replace the webwork with <webwork source="Library/this/problem/does/not/exist.pg"/>. Then run make mini-extraction and you should get an appropriate message terminating the extraction .

Take the minimal webwork example and delete a semicolon in the pg-code. Then run make mini-extraction and you should get an appropriate message terminating the extraction.

Take the minimal webwork example and replace the webwork with <webwork source="Library/Rochester/setDerivatives1/ns2_8_31.pg"/> which is a problem with elements that have not been given PTX output definitions. Then run make mini-extraction and you should get an appropriate message terminating the extraction.

rbeezer commented 5 years ago

This will be great to have. Generally, I would only like to totally bail out when it is impossible to proceed. We had a problem this summer where an author was hung-up for a while due to a problem (error) he couldn't solve. I mean to patrol the XSL message/@terminate='yes' one day to see how many I can liberalize. (Since I think there are existing examples contrary to what I say below.)

How about just baling out on the problem problem and proceeding from there?

Anyway, the point would be to ring lots of alarms and sound sirens, but don't put up a roadblock. Fail on the individual problem, not the whole process.

Alex-Jordan commented 5 years ago

What do you think about prompting with an option to bail or proceed? With the thousands of problems in ORCCA, I would prefer to bail, fix it, and start over. Rather than let the 30 minutes of subsequent ww extraction continue only to learn I have to repeat that.

rbeezer commented 5 years ago

The future is that the scripts will run un-attended on a server, so I'd like there to be no need for human interaction. Reports, yes; monitoring, no.

How about another (elective) step up in verbose-ness that will stop at severe errors? Eventually the script would benefit from a better hierarchy of warnings, but I don't think either of us wants to deal with that technical debt right now.

Alex-Jordan commented 5 years ago

I'm wrestling between what I know will work well for me with ORCCA (stopping cold on these WW issues) and your general desire to not bail and proceed. What about a switch? It could be general purpose. Something that says if you hit some of these issues that (will eventually) need attention anyway, stop? Maybe a switch like that could be useful outside of webwork processing too?

rbeezer commented 5 years ago

What about a switch? It could be general purpose.

Yes., that'd work, too. But default is off/procede. ;-)

Alex-Jordan commented 5 years ago

Any preference on name/abbreviation?

Like -t for terminate upon error. Or something like that?

rbeezer commented 5 years ago

Yep. Or -p for "pause"? I can't recall what's taken already.

Choose the most unlikely letter to be used for other purposes, but which still makes sense here. ;-)

On 12/9/18 6:40 PM, Alex Jordan wrote:

Any preference on name/abbreviation?

Like |-t| for terminate upon error. Or something like that?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rbeezer/mathbook/pull/980#issuecomment-445637455, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy2cvEd_REAA3rA0FFz0aOMIQiH9h6Fks5u3cmLgaJpZM4ZJ_V1.

Alex-Jordan commented 5 years ago

There are warnings and errors. To me, the things we are looking for here are errors, not warnings. Do you agree?

If you do, we are now about to classify these things as "recoverable errors". Meaning that the -a flag (for abort upon recoverable error) will apply to these. That could be different than an "error" that demands terminating the script.

Should the alert messages be like PTX:RECOVERABLE ERROR? Or something that distinguishes from PTX:ERROR?

rbeezer commented 5 years ago

On 12/10/18 2:44 PM, Alex Jordan wrote:

There are warnings and errors. To me, the things we are looking for here are errors, not warnings. Do you agree?

Yes.

If you do, we are now about to classify these things as "recoverable errors". Meaning that the |-a| flag (for abort upon recoverable error) will apply to these. That could be different than an "error" that demands terminating the script.

Sounds good.

Should the alert messages be like |PTX:RECOVERABLE ERROR|? Or something that distinguishes from |PTX:ERROR|?

Let's keep it as a "PTX:ERROR" for now. we can split hairs once somebody cares enough to put a proper warning system in place (there is a Python package called "warnings", iirc).

Alex-Jordan commented 5 years ago

OK, I just force pushed a new version. For testing:

  1. Should be no difference when making the sample chapter.
  2. In sample chapter, change some webwork to have an @source that is nonsense (not going to be an actual pg file on the server). Then make sample-chapter-extraction. You should see an error message scroll by, and then in the webwork-extraction.xml search for "failure".
  3. In sample chapter, change some webwork with pg-code to to break the Perl (eg, remove a needed semicolon). Then make sample-chapter-extraction. You should see an error message scroll by, and then in the webwork-extraction.xml search for "failure".
  4. In sample chapter, change some webwork to have an @source="Library/Rochester/setDerivatives1/ns2_8_31.pg" (which returns content that is not valid XML). Then make sample-chapter-extraction. You should see an error message scroll by, and then in the webwork-extraction.xml search for "failure".
  5. In Makefile, add the -a option to sample-chapter-extraction definition, and repeat items 2--4. The script should abort upon encountering any of these errors.
Alex-Jordan commented 5 years ago

When this boils down, possibly you will want to commit the -a option separately.

rbeezer commented 5 years ago
Alex-Jordan commented 5 years ago

I had to restore a closing quote on an attribute twice.

Oops, wasn't testing on merge. That area was fiddled with because I could no longer assume the absence of an attribute on the static at that point.

I've got the fixes and can mangage them, even under a force push.

OK, but if I do more here (as it looks like I will) I will fix that and make another force push. You don't mind trashing your branch and reconstituting? Would you prefer I just tack on commits here? (Sorry if you've answered this in the past.)

I see static[problem] being assigned a shell problem but it does not become the server-url version, so the live HTML just fails with the usual WW debugging. Maybe that is what you want? Can we insert the "shell" warning into the first part of the statement to indicate the problem is known by PTX?

I can do better things here. STay tuned for next iteration.

I'm not wild about the whole source getting spit out on the console.

I struggled with what to report when it is a PTX-authored problem. With a server problem, reporting the file name is obvious. With a PTX-authored problem, something like webwork-14 is not super helpful to locate it in your source. What do you think helps? Maybe just the converted pg?

rbeezer commented 5 years ago

Force push on your end is fine on my end.

Let's report the @xml:id of the containing exercise (which will encourage use of @xml:id) and then say something short like

Use -a to halt with full version of the problem.

and than make good on that advice? The goal is to reduce support questions by giving enough information for folks to find and correct their own errors, not to be super-convenient. ;-)

Alex-Jordan commented 5 years ago

Suppose you code your own problem in PTX source, but the Perl syntax is broken. Then you make the extraction (not seeing warnings), make the merge, and then make the export to .pg files. Which is more helpful? To make the .pg files with the broken syntax or to make one of these empty shell problems we are discussing that simply states the syntax was broken?

rbeezer commented 5 years ago

Reading the warnings is most helpful. If folks won't survey the putput, then they get what they get. ;-) Which is why we should be very economical with what we say. Warnings should stand out.

That said, don't put in a broken problem. Put in a shell version where it is obvious to any reader that there is a coding-problem to be reported.

I've got nothing bad to say about any of your suggestions or approaches, but I have ideas (a philosophy?) about how to communicate problems, and am trying to keep that consistent so that authors know what to expect.

Alex-Jordan commented 5 years ago

We don't have the ID of the containing exercise at present. We could get that, but would need to add it to one of the extraction style sheets. What we have is the internal ID of the webwork itself. Is that sufficient?

I hesitate to change things to get the exercise's ID, because 1) I am so far confined to mbx script with this, and it's nice to stay so contained 2) Are we sure webwork will only ever exist as a child of an exercise? What if it's a child of interactive, project, or something new some day? The less the code assumes an exercise is the parent, the fewer revisions might be needed some day.

rbeezer commented 5 years ago

On 12/10/18 9:47 PM, Alex Jordan wrote:

We don't have the ID of the containing exercise at present.

Right. Use the ID of the "webwork". Maybe suggest in the documentation that meaningful xml:id on the "webwork" will help with debugging trips to the server?

Alex-Jordan commented 5 years ago

OK, force pushed again. Same testing routine as before.

rbeezer commented 5 years ago

Looks great! This will be a big help for folks producing WW problems. I only tested the "no compile" scenario, but the code all looks good. Going up now.

(I couldn't split out the abort part without requiring a tedious fiddle to put your name back on the commits.)