- Notifications
You must be signed in to change notification settings - Fork1k
chore: move workspace apps tests to new package#7025
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| // RunTests runs the entire workspace app test suite against deployments minted | ||
| // by the provided factory. | ||
| func RunTests(t *testing.T, factory DeploymentFactory) { |
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.
Because this isjust used inworkspaceapps_test.go and isn't tested independently, should it be it's own package?
I know it's large in size, but I might argue that it should be in theworkspaceapps_test package instead, because that's what it's being used for anyways.
Regardless, we could call thisRun instead, sincetest is in the package name.
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.
I think we can move it toworkspaceapps pkg too.
Actually we cannot. You cannot import_test packages. So if we moved this toworkspaceapps directory andworkspaceapps_test package, then we cannot import it in our APGL and enterprise tests.
We will call this from the APGL and enterprise coderd tests.
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.
This is primarily moving code. LG, going to try and use this in enterprise now.
Moves almost every test from
workspaceapps_test.goand the reconnecting pty test to a new packagecoderd/workspaceapps/apptest.Closes#6905