-
Notifications
You must be signed in to change notification settings - Fork 24
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
Commit small test json files for each test instead of the large ones #509
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done!
The tests.json
files were removed in the ssv and types modules, but not in the qbft module. Is there a reason for that? It seems to me that all should be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
A few requests besides my comment:
- Can you pretty print the jsons into files. This will help with tracking changes without external tools
- I think it will be a good idea to add arg to mains so that by default will generate the big file only (to not change CI process). But with a given option will only generate small files. This will make us run faster.
Good job overall!
@alan-ssvlabs |
Thanks @GalRogozinski, updated the description and made the suggested changes But for the file permissions it fails the gosec because gosec suggests 600 or less, and it raises permission denied for the json files when using |
@alan-ssvlabs |
Did it as 644, gosec is not happy because it expect 600 or less so I added nosec. Also the permission denied messages are still there on git add... |
@alan-ssvlabs |
Very strangely I didn't get any permission errors before editing this 444 or 644 permissions, now no matter how I set it I get permission errors for both go generate and git add. I suspect it is my local problem, can you clone this branch and try to generate and git add? @GalRogozinski |
@alan-ssvlabs
I think what is happening is once the files were created once w.o write permission it is hard for them to be deleted. |
Seems I gotten myself to a position where I get permission errors always unless if I use the SUDO flag maybe we go back to original permissions if this causes issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should pretty print the jsons
Fixes #482
This PR creates one json file for each test. Large tests.json files are still generated but no longer committed.
The tests still use large tests.json files to run, the small json files are to keep track of changes easily.