fhir-crucible / testscript-engine

Ruby FHIR TestScript execution engine
Apache License 2.0
5 stars 2 forks source link

Quick fix to intake TestScripts with xml format #24

Closed jhlee-mitre closed 2 years ago

jhlee-mitre commented 2 years ago

Fixed load_scripts method to take xml formatted TestScript. I believe the current code was intended work that way, since the regular expression covers both json and xml

#{root}/**/*.{json, xml}

For some reason it doesn't work. So I modified as below.

(["#{root}/**/*.{json}", "#{root}/**/*.{xml}"])

Tested with TouchStone TestScripts (along with reverse converted TestScript from JSON to XML) and it works well.

Another finding: I tested XML formatted fixtures and it worked well without code modification.

ms-k1ngk0ng commented 2 years ago

Can you confirm my change works on your end? Then I will approve. Nice feature to add!

jhlee-mitre commented 2 years ago

I pulled your change and tested it and it didn't work. Looks like you put back the part I changed ->

#{root}/**/*.{json, xml}

It only captures json, not xml. Which is interesting. I checked Ruby spec and it should work. I switched xml and json their order, then it worked for xml but didn't capture json.

#{root}/**/*.{xml, json}

That was the reason why I changed the code though it's not beautiful.

(["#{root}/**/*.{json}", "#{root}/**/*.{xml}"])

Can you shortly try on your end and see it works differently?

jhlee-mitre commented 2 years ago

Another commit to address the issue and add examples

jhlee-mitre commented 2 years ago

To test, run command:

ruby driver.rb search_test_script.xml 
ruby driver.rb read_test_script.xml 
ms-k1ngk0ng commented 2 years ago

pushed change that renames the TestScripts for consistency. If looks good to you @jhlee-mitre, its good for merge.