Skip to content
This repository has been archived by the owner on Apr 13, 2021. It is now read-only.

Retire Perl/Python schema validation #93

Merged
merged 2 commits into from
Jul 1, 2019

Conversation

HebaruSan
Copy link
Member

The bot currently downloads ckan-validate.py and CKAN.schema and runs them on each .ckan file created or changed.

As of KSP-CKAN/CKAN#2788, validation against the schema is done by netkan.exe itself. If the generated metadata doesn't obey the schema, an error will be printed and the file won't be generated. This means that the bot's Perl/Python validation is now redundant.

This pull request removes the validation logic from the bot, including dependency installation, configuration, tests, etc.

@coveralls
Copy link

coveralls commented Jun 27, 2019

Coverage Status

Coverage decreased (-0.7%) to 90.959% when pulling ab645c5 on HebaruSan:remove-validation into 1fdc13f on KSP-CKAN:master.

@DasSkelett
Copy link
Member

Well, you just removed a whole test+validation framework, no wonder code coverage decreases... 😄

@techman83
Copy link
Member

One of the other tests failed, not sure why yet. One of the stage tests failed for some reason:

➜  NetKAN-bot git:(711943c) prove -lv t/App/KSP_CKAN/NetKAN.t 
t/App/KSP_CKAN/NetKAN.t .. 
ok 1 - use App::KSP_CKAN::NetKAN;
ok 2 - NetKAN Inflated
ok 3 - We commited files to master
ok 4 - We started on the master branch
ok 5 - Staged netkan not commited to master
ok 6 - We are on the DogeCoinFlagStaged branch
not ok 7 - We commited files to DogeCoinFlagStaged

#   Failed test 'We commited files to DogeCoinFlagStaged'
#   at t/App/KSP_CKAN/NetKAN.t line 63.
ok 8 - NetKAN cache path set correctly
ok 9 - no (unexpected) warnings (via done_testing)
1..9
# Looks like you failed 1 test of 9.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/9 subtests 

Test Summary Report
-------------------
t/App/KSP_CKAN/NetKAN.t (Wstat: 256 Tests: 9 Failed: 1)
  Failed test:  7
  Non-zero exit status: 1
Files=1, Tests=9, 14 wallclock secs ( 0.02 usr  0.00 sys +  2.12 cusr  0.38 csys =  2.52 CPU)
Result: FAIL

The staging process is a bit on the complicated side, so I'll have to take some time before I can merge this. Though I might get to finishing up the new indexer before that. I've actually replaced a huge amount of the functionality already as the new process is far far simpler and isn't required to be as defensive as NetKAN is very reliable with its output now.

@HebaruSan
Copy link
Member Author

Guess what's causing that error? 😏

$ ~/github/CKAN/_build/netkan.exe DogeCoinFlagStaged.netkan
420 [1] FATAL CKAN.NetKAN.Program (null) - spec_version v1.10+ required for install with 'find_regexp'

I'll fix it up in this PR...

@HebaruSan
Copy link
Member Author

I also want to state for the record that the logs on ci.ksp-ckan.space are much more user friendly than travis-ci.org. Excellent work on that!

@techman83
Copy link
Member

techman83 commented Jul 1, 2019

Build passes smoke tests locally, merging with thanks! Also, good catch! whistles quietly at self fail

@techman83 techman83 merged commit 4f218a2 into KSP-CKAN:master Jul 1, 2019
@HebaruSan HebaruSan deleted the remove-validation branch July 1, 2019 02:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants