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

Define a specified test file naming convention - *.test.yaml #131

Open
dylanhitt opened this issue Jun 28, 2020 · 5 comments · May be fixed by #206
Open

Define a specified test file naming convention - *.test.yaml #131

dylanhitt opened this issue Jun 28, 2020 · 5 comments · May be fixed by #206
Labels
enhancement New feature or request refactor

Comments

@dylanhitt
Copy link
Member

dylanhitt commented Jun 28, 2020

Define naming conventions for file, and only run commander against those files. Ex.

  • *.test.yaml
  • *.cmd_test.yaml
  • *.commander.yaml

--dir currently panics when an invalid file type is parsed. Proposed changes:

  1. Skip over all invalid files (i.e .sh, .bash, .yaml, etc).
This was referenced Jun 28, 2020
@SimonBaeumer
Copy link
Member

  1. The error message should contain the file name to better debug it. Error handling, especially parsing errors, are another huge issue on its own which we could discuss in a separate issue imho.
  2. Could you explain this a little bit more? I assume you mean special case yaml configs like k8s resources?
  3. I would go for this option, because it would not have any value to panic on different file types.
  4. This wouldn't be necessary if we just skip invalid file types :)

@dylanhitt
Copy link
Member Author

dylanhitt commented Jul 2, 2020

  1. I agree, that could easily be its own effort
  2. You pretty much hit it on it's head. Currently we support just yaml, json eventually. It's very easy accomplish number 3. My proposal is to throw those files say k8s resources, docker-compose stacks, anything yaml into Result.Warnings so at the end of exec we can provide the user with something. Let's say we have a dir with 4 files two of witch are commander configs and 2 are k8s resources, and one of the commander configs is invalid. It would ve nice to provide a warning that states [filename]: invalid commander config file or something along those lines. It may be a good warning to suggest that users move their asset files to separate dirs.
  3. Nice
  4. I was thinking it would be nice to just get out of the way, but yeah we can avoid the second loop.

@SimonBaeumer
Copy link
Member

You pretty much hit it on it's head. Currently we support just yaml, json eventually. It's very easy accomplish number 3. My proposal is to throw those files say k8s resources, docker-compose stacks, anything yaml into Result.Warnings so at the end of exec we can provide the user with something. Let's say we have a dir with 4 files two of witch are commander configs and 2 are k8s resources, and one of the commander configs is invalid. It would ve nice to provide a warning that states [filename]: invalid commander config file or something along those lines. It may be a good warning to suggest that users move their asset files to separate dirs.

Handling special cases is a tough topic. I try to break your idea ma little bit down to see which problems could occur in the long run:

  • How differentiate between k8s resources and commander configs?
    • i.e. on apiVersion root element, but what if the k8s resource was messed up too?
  • How differentiate between all the other yaml configs a user could provide?
    • a lot of special cases and issues are incoming
  • If we provide this feature to yaml configs, what about json in the future?
  • Do we have a reason to enforce putting assets into separate dirs?
    • This could lead to frustration, if you use this tool it should work right away. If a structure is enforced it will get more complex and annoying. There are a lot of tools/languages which are too hard to use, i.e. java or even go (remember the gopath replaced more or less by go modules).

This problem was already solved by other testing frameworks like the testing package in go or the *Unit frameworks like phpunit or junit.
We could just say we only execute files with a special suffix, i.e. kubernetes_resource.test.yaml.
Suffixes I would suggest:

  • *.test.yaml
  • *.cmd_test.yaml
  • *.commander.yaml

I would prefer the *.test.yaml as the default. A cli argument could make it configurable if *.test.yaml would not fit your use-case.
Here root config files could be useful to configure a test run on a directory, opened #136.

This is a breaking change if it would be required, thus a new major version would be needed. (imho no problem)

@dylanhitt
Copy link
Member Author

Awesome idea. Let's go with that. That also makes walking dirs much much simpler issue to address.

I personally would like to hold off on this and finish the other features/refactor we've talked about in other issues. Finishing #126 would make this a lot simpler as we won't have to deal with the goroutine in Test_Command.go anymore.

@dylanhitt dylanhitt added enhancement New feature or request refactor labels Jul 10, 2020
@dylanhitt dylanhitt changed the title Validate test dir before execution. Define a specified test file naming convention - *.test.yaml Jul 10, 2020
@dylanhitt
Copy link
Member Author

Instead of having to bump the version. Should we make this a feature and throw a deprecation warning for a few releases?

@taigrr taigrr linked a pull request Nov 2, 2023 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants