-
Notifications
You must be signed in to change notification settings - Fork 8
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
Attach diagnostics to tests #11
Comments
How does forcing the producer to always spit out diagnostics fix the issue? Sorry if I am not getting it, perhaps further explanation or examples might help. |
Because that way, you always know when to expect diagnostics: after a failure. |
I have always just checked each line for a directive, held onto the line and on the next pass through if it doesn't match the directive add diagnostics to the previous line as a document (not parsing the document). Again I feel I am missing you point though. |
You have to wait until the next result before you can process it. Which is IMHO against the streaming philosophy of TAP, as well as being an annoying processing complication. |
@Leont I'm not really sure how you could solve this 100% without making it all on one line. Much like the seconds of buffer for digital TV, I think it is legitimate for something consuming the TAP to always be delayed until the end of the test description, which means either:
However to potentially simplify this the ... or similar could end the line of the test:
However I'm not really a fan of that output. |
Just wanted to point out that the dev release of Test-Simple will make it easy to implement whatever is decided here. Internally diagnostics are now attached to the ok's that produced them (this is when they should be, some diags are independent). However old tools will need to update in order to attach their diags, so there could be a long process for tools to join the program. |
I'm still back to not understanding the advantage of this. Yes stricter output is something a producer might always want to trigger but variable length of YAML it doesn't help predict the number of tests or help the parser assume text positions. |
@Leont it took me some time to understand the issue here. IIUC, when streaming tests, there's no way to guarantee that when the parser finds a diagnostic, this diagnostic must be attached to the last test result found. So if we have the following TAP stream:
The following scenario with two threads could happen:
Here, there's no way to know whether the diagnostic should be attached to test 1 or to test 2. Correct me if I'm wrong @Leont. How about we define in the schema of diagnostics (I think we mentioned that somewhere here in GitHub) an entry to link back to the test result? Something like:
Just food for thought |
See #9 |
Currently, tests and diagnostics are entirely decoupled from a parsing point of view. The obvious approach of trying to parse a diagnostic after a test line fails is not streaming friendly, which is rather undesirable.
We should solve this somehow. Possible solutions are:
Probably there are other viable approaches too. I think I'd prefer option 3.
The text was updated successfully, but these errors were encountered: