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

Check yaml file syntax only once to save some time #82

Open
vnadot opened this issue Jan 10, 2024 · 4 comments
Open

Check yaml file syntax only once to save some time #82

vnadot opened this issue Jan 10, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@vnadot
Copy link
Contributor

vnadot commented Jan 10, 2024

In suite mode, you can instantiate several yaml. For each yaml file, WeTest checks the syntax for each file. If you call the same file n times (example below), WeTest checks the same file n times. Some time could be saved here. Would it be possible to check the syntax only one time for the same yaml files ?

$ cat suite_epicsComGen.yaml 
version: {major: 2, minor: 0, bugfix: 0}

include:
  - ['epicsComGen_WeTest_bo_range.yaml', PV_NAME: '${P}GenTestModeCmd', PV_NAME_RB: '${P}GenTestModeRb', ZNAM: 'operation', ONAM: 'test', delay: 0.001] # sec
  - ['epicsComGen_WeTest_bo_range.yaml', PV_NAME: '${P}GenIocStartedCmd', PV_NAME_RB: '${P}GenIocStartedRb', ZNAM: 'IOC not started', ONAM: 'IOC started', delay: 0.001] # sec
  - ['epicsComGen_WeTest_bo_range.yaml', PV_NAME: '${P}GenHealthIocStartedDisaCmd', PV_NAME_RB: '${P}GenHealthIocStartedDisaRb', ZNAM: 'enable', ONAM: 'disable', delay: 0.001] # sec
  - ...
$ ./wetest suite_epicsComGen.yaml -m P=SL-SCL-CM1:RF-RFFI-1: --no-gui --force-play --no-pv --no-pdf-output
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_bo_range.yaml
Validation of YAML scenario file: epicsComGen_WeTest_ao_range.yaml
@vnadot vnadot added the enhancement New feature or request label Jan 10, 2024
@gohierf
Copy link
Contributor

gohierf commented Jan 11, 2024

I did not check the code, but my take on this is that is might not be a good idea.
Because when you instantiate several times the same suite file, you can do so with different MACRO.
I suppose a bad macro value could break the YAML schema.

So one instantiation might be OK and the next not, with the same file but different macros.

Yet I am not sure whether the check is actually performed after macro substitution... Which might be an actual bug in itself if it is not.

This is off the top of my head, no checking in the code. I let you guys dig to confirm or invalidate this position.

@vnadot
Copy link
Contributor Author

vnadot commented Jan 12, 2024

Thanks for your comment, indeed it is a good point. We need to discuss about this whith @minijackson

@minijackson
Copy link
Collaborator

I agree with our contributor from far away, the macros can change the type of a YAML element.

For example:

macros:
  a:
    - b
    - c

config:
  name: "$a"

and importing it with:

imports:
  - ["./a.yaml", {a: "hello"}]

Should lead to a validation error when loaded as-is, but is valid when imported through the second file.

We should be able to read it once, and parse it once, though, before pre-processing it and applying macros.

@vnadot do you have some critical performance issues?

@vnadot
Copy link
Contributor Author

vnadot commented Sep 26, 2024

For testing all my variables mapped on FPGA registers, the syntax validation takes 1min25 for ~780 yaml called in the suite yaml file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants