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

[FIX] Installer allowing invalid config.yaml files #77

Merged
merged 3 commits into from
Apr 26, 2024

Conversation

suvanbanerjee
Copy link
Contributor

@suvanbanerjee suvanbanerjee commented Apr 26, 2024

uninstall: false
method: xyz
channel: abc
profile: ovos
features:
  skills: true
  gui: true
rapsberry_pi_tuning: true
share_telemetry: true

is an invalid config.yaml file but still is considered valid and installs ovos.

Fix: used conditional statements to validate each options correctness.
Closes: #72

@goldyfruit
Copy link
Member

goldyfruit commented Apr 26, 2024

@suvanbanerjee Thanks.

  • Did you test it?
  • What is the output when it breaks?

@goldyfruit goldyfruit added the bug Something isn't working label Apr 26, 2024
@goldyfruit goldyfruit added this to the Quake milestone Apr 26, 2024
@suvanbanerjee
Copy link
Contributor Author

suvanbanerjee commented Apr 26, 2024

Did you test it?

I tested it locally by running it not usings the BATS tests. and it works by saying seceniro not supported
image
log https://sprunge.us/ZvSMFd

What is the output when it breaks?

image
this is the output when using method: xyz and channel: abc in the current version

@suvanbanerjee
Copy link
Contributor Author

@goldyfruit I think the failed bats test in #73 is also due to the invalid config.yaml file. but unable to confirm as i am not aware how to run BATS test locally

@goldyfruit
Copy link
Member

@goldyfruit I think the failed bats test in #73 is also due to the invalid config.yaml file. but unable to confirm as i am not aware how to run BATS test locally

Just to make sure, your file is named scenario.yaml and not config.yaml, right?

@suvanbanerjee
Copy link
Contributor Author

@goldyfruit I think the failed bats test in #73 is also due to the invalid config.yaml file. but unable to confirm as i am not aware how to run BATS test locally

Just to make sure, your file is named scenario.yaml and not config.yaml, right?

my bad it's scenario.yaml only. 😅

https://pastebin.com/wV0AkayA here is the full log for the output when using method: xyz and channel: abc in the current version

@goldyfruit
Copy link
Member

I'm gonna test it later today. 👍

@suvanbanerjee
Copy link
Contributor Author

I'm gonna test it later today. 👍

Just tested it using github actions and it is still failing the bats test.

@goldyfruit goldyfruit merged commit c711226 into OpenVoiceOS:main Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle scenario existing file but empty
2 participants