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

Validate WeBWorK XML output #980

Closed
wants to merge 1 commit into from

Conversation

Alex-Jordan
Copy link
Contributor

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
Copy link
Collaborator

rbeezer commented Dec 9, 2018

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?

  • PTX:ERROR at the console at any/least verbose level (ie always).
  • Manufacture a shell problem that indicates the error. In other words, the problem would say "This WeBWorK problem is not available since " or similar. You could dress it up prominently.
  • Maybe put a new attribute on "static", like @compile=... with success, empty, incompatible, ... The "author's report" that I will be handing to Oscar could report these.

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

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
Copy link
Collaborator

rbeezer commented Dec 9, 2018

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

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
Copy link
Collaborator

rbeezer commented Dec 9, 2018

What about a switch? It could be general purpose.

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

@Alex-Jordan
Copy link
Contributor Author

Any preference on name/abbreviation?

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

@rbeezer
Copy link
Collaborator

rbeezer commented Dec 10, 2018 via email

@Alex-Jordan
Copy link
Contributor Author

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
Copy link
Collaborator

rbeezer commented Dec 10, 2018 via email

@Alex-Jordan
Copy link
Contributor Author

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

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

@rbeezer
Copy link
Collaborator

rbeezer commented Dec 11, 2018

  • I had to restore a closing quote on an attribute twice. One fixed line looks like
static[problem] = static[problem].replace('<static', '<static seed="' + seed[problem] + '" ')

Otherwise the creation of the merge file failed. I've got the fixes and can mangage them, even under a force push.

  • 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'm not wild about the whole source getting spit out on the console. Could we reserve that for the -a behavior? (Have not tested that mode yet.)

@Alex-Jordan
Copy link
Contributor Author

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
Copy link
Collaborator

rbeezer commented Dec 11, 2018

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

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
Copy link
Collaborator

rbeezer commented Dec 11, 2018 via email

@Alex-Jordan
Copy link
Contributor Author

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
Copy link
Collaborator

rbeezer commented Dec 11, 2018 via email

@Alex-Jordan
Copy link
Contributor Author

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

@rbeezer
Copy link
Collaborator

rbeezer commented Dec 12, 2018

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.)

@rbeezer rbeezer closed this Dec 12, 2018
@Alex-Jordan Alex-Jordan deleted the webwork-warnings branch September 14, 2023 03:24
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.

None yet

2 participants