Make 'TEST' the only proper mandatory test setting #1126
Conversation
|
ok, I think I made clear enough that I consider this wrong. If you can't follow, let's get old over this one here. |
Codecov Report
@@ Coverage Diff @@
## master #1126 +/- ##
==========================================
+ Coverage 87.01% 87.02% +<.01%
==========================================
Files 101 101
Lines 7392 7396 +4
==========================================
+ Hits 6432 6436 +4
Misses 960 960
Continue to review full report at Codecov.
|
|
the settings are currently set as In short, I see the following options:
I prefer 1, I can also follow 2 if you like. |
|
For POST /api/v1/isos they are required. POST /api/v1/jobs doesn't require them, but IMO that's mistake. |
|
3 is what I want, I can agree to 2. |
okurz
force-pushed
the
feature/no_mandatory_job_settings
branch
from
1c16f5d
to
d585c7d
Compare
|
Looks like a rebase needed |
okurz
force-pushed
the
feature/no_mandatory_job_settings
branch
from
d585c7d
to
20aab43
Compare
|
updated to follow approach "3", i.e. initialize values in the database with an empty string except for TEST which is now mandatory (as in the previous commits in this PR). open point: I wrote the upgrade script https://github.com/os-autoinst/openQA/pull/1126/files#diff-dd139f35469d0636667b588967ab0c64 with @Martchus but checking on a local database it is either not called or has no effect on migration. How can I ensure the script is called before the schema is upgraded and not after the try to migrate the database which would fail because there are entries which are not allowed anymore after the schema upgrade? |
|
not calling 01 but a different number |
|
I.e. if you want to run it before the DB migration, name it 01 and the sql 02 |
In before there was no real check for most settings implicitly allowing undefined settings but causing perl warnings. This commit now properly tests job results where no setting is set besides the name composed of 'TEST' itself. It is not pretty in all cases when the individual string components are empty but it works. This commit with the according database schema upgrade makes the values DISTRI VERSION FLAVOR ARCH BUILD required in the schema with default to empty strings. Also, there is a database migration script to find all current undef values in the database and update to empty string. The in-place job settings migration and transfer on update is updated accordingly to avoid explicitly overwriting the values with undef in the database which would null the entries which is not allowed anymore. Instead, the database takes care to provide a valid empty string as value in the according cases. Not fully covered is: * handle it in UI accordingly where needed e.g. to prevent strings like '--.' * review the API entry points for additional checks for these settings and harmonizing them Related progress issue: https://progress.opensuse.org/issues/6762
okurz
force-pushed
the
feature/no_mandatory_job_settings
branch
from
20aab43
to
860baf1
Compare
|
updated:
I managed to still show warnings when showing some weirdly created tests locally but so far couldn't find the culprit. Anyway, a related fix would probably not require database changes and also tests should pass now already so IMHO ready to be merged although I would not mind if anyone else tests manually as well. |
In before there was no real check for most settings implicitly allowing
undefined settings but causing perl warnings. This commit now properly
tests job results where no setting is set besides the name composed of
'TEST' itself. It is not pretty in all cases when the individual
string components are empty but it works.
Related progress issue: https://progress.opensuse.org/issues/6762