Skip to content

Comments

update PR tests to run same as deploy time #7157

Merged
Jskobos merged 3 commits intolinode:developfrom
stvnjacobs:ci/test-deploy-one-workflow
Dec 16, 2020
Merged

update PR tests to run same as deploy time #7157
Jskobos merged 3 commits intolinode:developfrom
stvnjacobs:ci/test-deploy-one-workflow

Conversation

@stvnjacobs
Copy link
Contributor

Description

This will flag off deploy steps from the workflow to only run on the
master branch. This way the same tests will run on each PR that will be
run before release.

Type of Change

  • Bug fix ('fix', 'repair', 'bug')

This will flag off deploy steps from the workflow to only run on the
master branch. This way the same tests will run on each PR that will be
run before release.
This is now handled by github action workflows
@stvnjacobs stvnjacobs changed the title Ci/test deploy one workflow update PR tests to run same as deploy time Nov 24, 2020
@stvnjacobs
Copy link
Contributor Author

Another option: We could also split this into two workflows if we don't want to see the skipped tests. In that configuration, the workflow triggered by pull requests would run tests, while the workflow triggered my commits to master would run the builds and deployments.

For future consideration: I also think it may be soon time to consider releasing based off tags instead of every merge to master. In this scenario, we could get rid of the development branch, test PRs as they go into master, and deploy when a version is tagged. It cleans up this pipeline a bit, and also is easier to maintain.

# https://stackoverflow.com/questions/58033366/how-to-get-current-branch-within-github-actions#comment102508135_58034787
build-manager:
runs-on: ubuntu-latest
if: github.ref == 'refs/heads/master'
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this expression be true if a pull request to master is opened?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the ref would be the current branch, which in this case would be the branch name of the users branch.

https://docs.github.com/en/free-pro-team@latest/actions/reference/context-and-expression-syntax-for-github-actions#github-context

Please do double check me on that. I searched around a bit to find the best way to only run on changes to master, and this was pretty common.

@johnwcallahan
Copy link
Contributor

Another option: We could also split this into two workflows if we don't want to see the skipped tests. In that configuration, the workflow triggered by pull requests would run tests, while the workflow triggered my commits to master would run the builds and deployments.

I like the idea of always running the tests in the same workflow as the deployment. That way the tests are run if someone pushes directly to master without a PR (which is how we currently do our release).

Personally the skipped checks don't bother me. What'd be really great is if we could have the SDK build + tests as its own workflow that is also run as part of the deployment workflow, but it doesn't seem like that's possible. (In effect that's what's happening here, but it'd be cool if we could put it in its own workflow file).

For future consideration: I also think it may be soon time to consider releasing based off tags instead of every merge to master. In this scenario, we could get rid of the development branch, test PRs as they go into master, and deploy when a version is tagged. It cleans up this pipeline a bit, and also is easier to maintain.

Yes.

Copy link
Contributor

@Jskobos Jskobos left a comment

Choose a reason for hiding this comment

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

Thanks for getting this up so quickly. looks good, and I'm totally on board with removing develop

@stvnjacobs
Copy link
Contributor Author

Last thing, do we want to build manager on pull requests to make sure it builds? Are there any instances where tests would pass but it would not build successfully?

@Jskobos
Copy link
Contributor

Jskobos commented Dec 2, 2020

Last thing, do we want to build manager on pull requests to make sure it builds? Are there any instances where tests would pass but it would not build successfully?

Don't we want to deploy the build artifact as we do currently through Jenkins? In general, building isn't critical as long as typechecking and the linter pass. Which...we could include in test-manager but currently don't. If building every PR is going to be hard on the infrastructure or our account limits we can skip it, but it's helpful in general.

@Jskobos Jskobos merged commit 442d2c1 into linode:develop Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants