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

OSOE-770: Add support for structured html-validate output #754

Merged
merged 19 commits into from May 12, 2024

Conversation

AydinE
Copy link
Contributor

@AydinE AydinE commented May 3, 2024

@AydinE
Copy link
Contributor Author

AydinE commented May 3, 2024

Hi @DemeSzabolcs ,

I created a new PR because I was having some issues with properly merging submodules etc. on the old PR, seemed easier to just create a new one.

With some extra testing today I think I figured out the issue. Looking at the logging it seems that perhaps there is a max length for the output string because when I log the string in the exception the JSON just cuts off (see output of latest failed run) because of this it's not valid JSON anymore.

The easiest way would to instead use the outputfile instead of the output string like I was doing before, but that time you made a comment that the outputfile was not nicely human readable see comment: Lombiq/UI-Testing-Toolbox#354 (comment)

Seems like though using the outputfile as JSON is the most reliable option though. let me know your thoughts.

@DemeSzabolcs
Copy link
Member

Hi @DemeSzabolcs ,

I created a new PR because I was having some issues with properly merging submodules etc. on the old PR, seemed easier to just create a new one.

With some extra testing today I think I figured out the issue. Looking at the logging it seems that perhaps there is a max length for the output string because when I log the string in the exception the JSON just cuts off (see output of latest failed run) because of this it's not valid JSON anymore.

The easiest way would to instead use the outputfile instead of the output string like I was doing before, but that time you made a comment that the outputfile was not nicely human readable see comment: Lombiq/UI-Testing-Toolbox#354 (comment)

Seems like though using the outputfile as JSON is the most reliable option though. let me know your thoughts.

Hi! @AydinE

First I would suggest trying it out and see if it works. (So that one test no longer fails).

use the outputfile instead of the output string like I was doing before

If using the outputfile works (so it fixes that test), then:
Isn't there a way, to use the outputfile for the validation (so there will be no cutoff), but use the output string for the actual logging (so the message would be still human readable)?

@AydinE
Copy link
Contributor Author

AydinE commented May 6, 2024

Hey @DemeSzabolcs ,

After some more debugging inside of the pipeline it seems like the issue is also present when using the output file. For some reason when running locally this is not an issue but there seems to be a specific limit for files generated by the cli tool in the pipeline. This makes it so that the file gets cut off in the middle and is no longer valid JSON so can not be parsed.

The reason the file is so large is that for some reason the html-validate library includes the source property inside the JSON format which logs the html source inside of the JSON object - which it does not do for the other output formatters e.g. styling or text.

For now I have been able to "solve" this by making sure there is still valid JSON if the file gets cut off.
See: Lombiq/UI-Testing-Toolbox@361ab7e...9e23b39#diff-67847fe58db929ae56cd24e4ba914c28af919cdf0272fd3cbf193c995d2666aeR51 - however because of the cutoff there actually may be valid errors that we are missing in this situation so this is not really a viable option
Because we unfortunately don't have more control over what the output looks like that html-validate outputs when using the JSON formatter there also does not seem to be a way to exclude the source from the object.

The only real way to solve it is to figure out why the cutoff happens in the pipeline, but I am not even sure what the steps would be or if this would even be in our control.

Would like to know what your thoughts are on how to continue with this.

Copy link

github-actions bot commented May 6, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

1 similar comment
Copy link

github-actions bot commented May 6, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

@DemeSzabolcs
Copy link
Member

Hey @DemeSzabolcs ,

After some more debugging inside of the pipeline it seems like the issue is also present when using the output file. For some reason when running locally this is not an issue but there seems to be a specific limit for files generated by the cli tool in the pipeline. This makes it so that the file gets cut off in the middle and is no longer valid JSON so can not be parsed.

The reason the file is so large is that for some reason the html-validate library includes the source property inside the JSON format which logs the html source inside of the JSON object - which it does not do for the other output formatters e.g. styling or text.

For now I have been able to "solve" this by making sure there is still valid JSON if the file gets cut off. See: Lombiq/UI-Testing-Toolbox@361ab7e...9e23b39#diff-67847fe58db929ae56cd24e4ba914c28af919cdf0272fd3cbf193c995d2666aeR51 - however because of the cutoff there actually may be valid errors that we are missing in this situation so this is not really a viable option Because we unfortunately don't have more control over what the output looks like that html-validate outputs when using the JSON formatter there also does not seem to be a way to exclude the source from the object.

The only real way to solve it is to figure out why the cutoff happens in the pipeline, but I am not even sure what the steps would be or if this would even be in our control.

Would like to know what your thoughts are on how to continue with this.

I would play around with the formatters: Lombiq/UI-Testing-Toolbox@361ab7e...9e23b39#diff-a24e86c8d98936bee437d7e731504a5cc95e4e36d38f5c4c644574177747a082R34-R35

And meanwhile try to figure out why the cutoff happens (especially on GitHub but not locally).

If I'm seeing it right, the last part is the source, so the errors are on the beginning of the output, so cutoff of errors would be unlikely, but it's still annoying that a cutoff happens.

This comment has been minimized.

@AydinE
Copy link
Contributor Author

AydinE commented May 7, 2024

Hey @DemeSzabolcs ,

I played around with this some more today and was actually able to reproduce this locally on a Ubuntu install aswell using the JSON output formatter. Unfortunately I think that the issue might be out of our control since perhaps this is to do with how atata framework reads the output of invoking the html-validate command. Perhaps it's worth creating a ticket over on their repo for it?

For now though I think the current workaround will do because as you said the source object is all the way at the bottom of the main JSON object so the actual errors would not get lost and we are not really doing anything with the source output anyway so we don't really care about it.

Let me know what you think otherwise I believe this one is ready for a review.

@DemeSzabolcs
Copy link
Member

Hey @DemeSzabolcs ,

I played around with this some more today and was actually able to reproduce this locally on a Ubuntu install aswell using the JSON output formatter. Unfortunately I think that the issue might be out of our control since perhaps this is to do with how atata framework reads the output of invoking the html-validate command. Perhaps it's worth creating a ticket over on their repo for it?

For now though I think the current workaround will do because as you said the source object is all the way at the bottom of the main JSON object so the actual errors would not get lost and we are not really doing anything with the source output anyway so we don't really care about it.

Let me know what you think otherwise I believe this one is ready for a review.

At least we now know that this only happens on Ubuntu. Please create a ticket about this in the corresponding repository.

Yes, I don't have a better idea, I think it's okay for now. However maybe add a note in the code about this (or if I remember you already added one), but please also link the ticket that you have created. I will review the PRs in the upcoming days.

@AydinE
Copy link
Contributor Author

AydinE commented May 8, 2024

Yes, I don't have a better idea, I think it's okay for now. However maybe add a note in the code about this (or if I remember you already added one), but please also link the ticket that you have created. I will review the PRs in the upcoming days.

I did already add a note about this but I have also added a link to the issue for easy tracking.
atata-framework/atata-htmlvalidation#9

@DemeSzabolcs DemeSzabolcs merged commit 227f477 into dev May 12, 2024
18 checks passed
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

3 participants