Skip to content

fix(cli): template pull tests: await template version job before exiting #9430

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

Merged
merged 2 commits into from
Aug 31, 2023

Conversation

johnstcn
Copy link
Member

Seen here: https://github.com/coder/coder/actions/runs/6022883220/job/16338551690?pr=9421#step:5:354

Ignoring logger errors here as we have assertions on the template data to verify success.

@johnstcn johnstcn requested a review from spikecurtis August 30, 2023 10:32
@johnstcn johnstcn self-assigned this Aug 30, 2023
@spikecurtis
Copy link
Contributor

Ignoring logging errors is pretty broad.

Here what's going on is that we're closing down the test before the template version job is finished, and the provisioner is throwing an error. If we wait for the job to finish, we shouldn't see an error if everything is working.

It's also maybe a little surprising to me that if you push a new template version and then immediately pull before the dry-run is complete, you get the new, possibly-failing version?

@johnstcn johnstcn changed the title fix(cli): ignore log errors in template pull tests fix(cli): TestTemplatePull.*: await template version job before exiting Aug 31, 2023
@johnstcn
Copy link
Member Author

@spikecurtis I admit I went for the 'quick fix' here and not necessarily the correct one. Updated the tests to await template version job.

@johnstcn johnstcn changed the title fix(cli): TestTemplatePull.*: await template version job before exiting fix(cli): template pull tests: await template version job before exiting Aug 31, 2023
@johnstcn johnstcn requested a review from mafredri August 31, 2023 10:15
@mafredri
Copy link
Member

It's also maybe a little surprising to me that if you push a new template version and then immediately pull before the dry-run is complete, you get the new, possibly-failing version?

Not sure whether or not that is a bug or if it's part of a separate issue that we, by default, return failed builds. This is also visible in the dashboard where you can use the template editor, have a syntax error -> failed build, and that new version will show up in the list.

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this looks like the appropriate fix for these tests. We might want to separately test what happens on failure though, but I think that's a separate issue.

@johnstcn johnstcn merged commit fd17857 into main Aug 31, 2023
@johnstcn johnstcn deleted the cj/flake/test_template_pull branch August 31, 2023 10:40
@github-actions github-actions bot locked and limited conversation to collaborators Aug 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants