BetaMasaheft / Documentation

Die Schriftkultur des christlichen Äthiopiens: Eine multimediale Forschungsumgebung
3 stars 3 forks source link

test "run failed" message / validation impossibility #2183

Open eu-genia opened 2 years ago

eu-genia commented 2 years ago

For each push to master I now get a run failed message even when the files have all been made valid e.g. https://github.com/BetaMasaheft/Works/actions/runs/2774202919 I do not know whether this is something to worry about or to ignore?

As we mention here https://github.com/BetaMasaheft/Documentation/issues/2174 and here https://github.com/BetaMasaheft/Documentation/issues/2169#issuecomment-1201023491 it will probably be never possible to have all files valid in the foreseable future. This was never a requirement and we cannot change the workflow at once.

I was not aware of the consequences of what your workflow would mean for us; is it possible that our normal work is not disrupted too much?

I am afraid that if every person trying to work with us keeps getting an email with an error message every time they try to push to master persons will be very quickly discouraged from contributing on BM at all.

@duncdrum @line-o @windauer

duncdrum commented 2 years ago

@eu-genia as you can see in the log, not all the files are valid there are still validation errors.

It was my understanding that you work via pull-request, instead of pushing to master. So to limit the noise from email messages we should stick with pull requests. Once a repo validates there won't be any warnings, unless someone pushed invalid files to master at which point we really would want to know about this, as these are currently automatically uploaded and deployed on the server where they can and do cause problems.

eu-genia commented 2 years ago

For quick fixes I push to master - and the few files which I pushed do validate, as we limit commits to up to 25 files. The log report contains validation errors from other files in the repo which I did not touch upon when committing. If the message would report only errors in the files being pushed it would be acceptable, otherwise we won't be able to work, as, as mentioned repeatedly, there is no chance that ALL files in ALL repos are valid, in some cases (e.g. IHA subcollections not yet present on GitHub) they were never meant to be.

eu-genia commented 2 years ago

@thea-m @DenisNosnitsin1970 note that we are now seemingly doomed to always receive error messages... I do hope this can be fixed as requested or we won't be able to work.

duncdrum commented 2 years ago

@eu-genia you and any contributors can disable email notifications for GitHub action runs by following these instructions https://docs.github.com/en/account-and-profile/managing-subscriptions-and-notifications-on-github/setting-up-notifications/about-notifications

eu-genia commented 2 years ago

I do not mind error messages as long as commits actually do go through - but we cannot request all the perons contributing to spend the time to explore GitHub.

Another issue here: the log reports e.g. this file LIT4547Yaityo but it is valid as far as my Oxygen is concerned. Same LIT4543Enqelf LIT4545Yaalam LIT4542Esubdenq

eu-genia commented 2 years ago

When we work with PR we still get the same test problem https://github.com/BetaMasaheft/Manuscripts/pull/1788 - I guess we can bypass branch protection and merge, but again, the test reports OTHER invalid files, which we CANNOT fix at the moment, not the files actually being pushed. This makes our workflow truly cumbersome and unpleasant. I am sorry but this should not be the scope of an improvement.

line-o commented 2 years ago

@eu-genia we want to improve things for you. So, we can definitely exclude files from validation (which would make sense for IHA sub collection if they would be added to this GitHub repository).

Regarding external data like IHA, there might be other options, which we should discuss before rushing to a conclusion which might complicate things in the future. This is especially true if IHA or other external data sources get updates.

It should be possible to limit to the validation to the data files changed in a commit that was pushed.

Another issue here: the log reports e.g. this file LIT4547Yaityo but it is valid as far as my Oxygen is concerned. Same LIT4543Enqelf LIT4545Yaalam LIT4542Esubdenq

I will do a local test against our validation and post the report here in order for us to better understand what we are dealing with.

eu-genia commented 2 years ago

It should be possible to limit to the validation to the data files changed in a commit that was pushed.

this would be great!

eu-genia commented 2 years ago

Authority Files are all valid but the test fails https://github.com/BetaMasaheft/Authority-Files/runs/7812683544?check_suite_focus=true

eu-genia commented 2 years ago

more to failure to validate:

-- files with x:included texts/editions pose validations problems, also in this report: https://github.com/BetaMasaheft/Studies/runs/7805450614?check_suite_focus=true In such cases I am not sure whether it is the schema or the data, and what should the fix be.

-- some files appear valid to me but still produce error report e.g. I believe to have fixed Works but the test https://github.com/BetaMasaheft/Works/runs/7804168315?check_suite_focus=true has failed (mentioned also here https://github.com/BetaMasaheft/Documentation/issues/2183)

-- I started adjusting the schema to take IHA values, but even the valid files report ISO Schematron errors I have not come across before image

maybe you know what the necessary fixes could be.

line-o commented 2 years ago

@eu-genia I have not come accross those either. Maybe @duncdrum has seen something like it? It does look like the schema itself or an include has a syntactical problem, though.

line-o commented 2 years ago

you can follow the progress on validation workflow update here #2192

eu-genia commented 2 years ago

IHA - there was an issue with the schema which I fixed, The other questions are still open though

line-o commented 2 years ago

Can we close this issue?

eu-genia commented 2 years ago

I still get failed test in Persons (two records I see no problems with) and Manuscripts (a number of records I see no problems with plus split ones); I would close this once I stop getting error messages

eu-genia commented 12 months ago

Dear all, once again I get a failed run report https://github.com/BetaMasaheft/Manuscripts/actions/runs/6664117863/job/18111144435

./Hamburg/Stabi/SHOr407.xml fails to validate
./Hamburg/Stabi/SHOr404.xml fails to validate
./Hamburg/Stabi/SHOr271a.xml fails to validate
./Hamburg/Stabi/SHOr410.xml fails to validate
./Grottaferrata/GAet8.xml fails to validate
./PrivateCollections/Germany/TFR/TFR003.xml fails to validate

I cannot see any problems with the files that had not been touched for a while and never caused problems before. Is it something connected to the latest schema update? Thank you!

duncdrum commented 12 months ago

@eu-genia yes this looks like legacy data invalidated after the schema update. If you validate them locally in oxygen you should get a more detailed explanation of what the problem is.

Main validation file: ~/Jinntec/betamasa/Manuscripts/Hamburg/Stabi/SHOr271a.xml
Schema: https://raw.githubusercontent.com/BetaMasaheft/Schema/master/tei-betamesaheft.rng
Engine name: Jing
Severity: error
Problem ID: invalid_attribute_value
Description: value of attribute "color" is invalid; must be a string matching the regular expression "(\p{L}|\p{N}|\p{P}|\p{S})+"
Start location: 238:73
End location: 238:86

Main validation file: ~/Jinntec/betamasa/Manuscripts/Hamburg/Stabi/SHOr404.xml
Schema: https://raw.githubusercontent.com/BetaMasaheft/Schema/master/tei-betamesaheft.rng
Engine name: Jing
Severity: error
Problem ID: invalid_attribute_value
Description: value of attribute "color" is invalid; must be a string matching the regular expression "(\p{L}|\p{N}|\p{P}|\p{S})+"
Start location: 214:73
End location: 214:84

Main validation file: ~/Jinntec/betamasa/Manuscripts/Hamburg/Stabi/SHOr407.xml
Schema: https://raw.githubusercontent.com/BetaMasaheft/Schema/master/tei-betamesaheft.rng
Engine name: Jing
Severity: error
Problem ID: invalid_attribute_value
Description: value of attribute "color" is invalid; must be a string matching the regular expression "(\p{L}|\p{N}|\p{P}|\p{S})+"
Start location: 283:73
End location: 283:85

Main validation file: ~/Jinntec/betamasa/Manuscripts/Hamburg/Stabi/SHOr410.xml
Schema: https://raw.githubusercontent.com/BetaMasaheft/Schema/master/tei-betamesaheft.rng
Engine name: Jing
Severity: error
Problem ID: invalid_attribute_value
Description: value of attribute "color" is invalid; must be a string matching the regular expression "(\p{L}|\p{N}|\p{P}|\p{S})+"
Start location: 267:73
End location: 267:95
eu-genia commented 12 months ago

The problem is caused by code such as <decoNote xml:id="b5" type="Endbands" color="brown black"> We need to enable multiple strings for the@color attribute