Fixes to test suite #1113
Fixes to test suite #1113
Conversation
| tokens := strings.Split(host, ".") | ||
| if len(tokens) == 0 { | ||
| return errors.Errorf("malformed hostname: %s", host) | ||
| ingressUrl := os.Getenv("ARMADA_EXECUTOR_INGRESS_URL") |
There was a problem hiding this comment.
It seems like you are missing the case of a bad hostname from host parameter.
Is that intentional?
There was a problem hiding this comment.
What do you mean?
There was a problem hiding this comment.
The original code parsed the host function parameter and returned an error if it couldn't get any tokens.
You are reading from an environment variable but if someone didn't specify it, you assume that the host function parameter is valid without any tokens.
You also don't check if the environment variable is a valid url spec.
There was a problem hiding this comment.
I think that's fine.
If the host name isn't set correctly, the test will fail, which is the correct behaviour.
If the env variable is set incorrectly, the test will also fail, which I think is fine.
There was a problem hiding this comment.
Kk. I didn't see that this was in test suite. Yea I think that's fine.
No description provided.