-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add 'vktest clear' command #2592
Conversation
mv "$STEPS_DIR/utterance_responses.py" $TMP_DIR | ||
rm ${STEPS_DIR}/*.py | ||
mv ${TMP_DIR}/* $STEPS_DIR | ||
rmdir $TMP_DIR |
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.
Below is nothing that needs to happen now, just something to consider.
We will likely need to expand the defined steps and will likely add more files here and remembering to update this file list will be tricky.
Can we make a change in the test_setup to prepend a skill_
to the files it pulls in? or rely upon this being run in a git repo and do a rm $STEPS_DIR/* and then a git checkout of the folder to get back all files?
The first option will likely result in issues of double definitions of steps if there are stepfiles in there now until the files have been manually deleted. Either people will have to delete their skills manually or we can check for the old name and remove it when pulling in the stepfiles from skills.
A third option may be having this script read a "manifest.txt" file in the steps folder listing the files that should remain. since it's in the directory it's easier to remember to update.
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.
Yeah, I was thinking about this. At the very least I'll add a comment about what the 2 files should be in the Features dir.
should we be appending a quasi-unique identifier for the skill it came from? eg
mycroft-npr-news-skill_filename.py
Just imagining people calling their custom steps file steps.py
meaning one would get overwritten.
I'm wary of baking in the expectation of it being a git repo, though it's very likely for anyone doing development.
Is there another way that test_setup
could track the files it's copying in, then remove each filename from the list as they're confirmed deleted?
The manifest.txt
seems like a simple middle ground.
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.
Yes, adding prefixes has been on my TODO for some time, collisions shouldn't occur
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.
Looks good and works as I'd expect. I made a note about the deleting but nothing that needs to be handled immediately.
The thing I found was that it should also remove files from the features folder ending in .config.json
to handle the new custom configuration values thing.
Voight Kampff Integration Test Failed (Results) |
1 similar comment
Voight Kampff Integration Test Failed (Results) |
Voight Kampff Integration Test Succeeded (Results) |
Voight Kampff Integration Test Succeeded (Results) |
Looks good. Merging. |
Description
Adds a new sub-command to
vktest
allowing developers to quickly clear any Skill Feature or Step files from the Voight Kampff directory.To avoid duplication, all the logic is contained in
bin/skill-test-runner
andstart-mycroft.sh
calls that.Had to do some file dancing to retain existing files. Any better suggestions welcomed...
How to test
Test a Skill then remove the files.
Contributor license agreement signed?