Skip to content

Conversation

@rlinehan
Copy link
Contributor

@rlinehan rlinehan commented Jan 12, 2022

Part of #3.

This adds a super basic happy path integration test using the unittest package. A new CI GitHub Action runs the tests on every pull request and on pushes to main.

Add a super basic happy path integration test using the `unittest` library.
Runs on every push and on pull requests.
@rlinehan rlinehan marked this pull request as ready for review January 12, 2022 22:32
@rlinehan rlinehan requested a review from gautamomento January 12, 2022 22:33
@rlinehan
Copy link
Contributor Author

@gautamomento Might want to chat with you about organization of the workflows in this repo - I'm not 100% sure if we need the On pull request and On push workflows; it might be better to organize as like CI and build or something like that? But not something that needs to be done in this PR, IMO.

gautamomento
gautamomento previously approved these changes Jan 13, 2022
@gautamomento
Copy link
Contributor

@gautamomento Might want to chat with you about organization of the workflows in this repo - I'm not 100% sure if we need the On pull request and On push workflows; it might be better to organize as like CI and build or something like that? But not something that needs to be done in this PR, IMO.

Yea sure thing lets chat about this.

Copy link
Contributor

@gautamomento gautamomento left a comment

Choose a reason for hiding this comment

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

The code looks good. But would like to better understand new ci.yml vs modifying existing. So, lets sync up on that.

@gautamomento gautamomento dismissed their stale review January 13, 2022 05:58

Accidentally clicked approve, wanted to comment instead.

@rlinehan
Copy link
Contributor Author

The code looks good. But would like to better understand new ci.yml vs modifying existing. So, lets sync up on that.

Yeah, to summarize my thoughts - the existing workflows are push_request and pull_request and are organized by what we should do for pushes vs. pull requests. But I think something like running tests we want to happen in both of those cases - on all prs and on at least pushes to main.

I think its more logical to group by type of thing the workflow is doing, similar to how the java sdk has ci.yml and cd.yml - so a workflow for running tests and a workflow for building and pushing the package.

@gautamomento
Copy link
Contributor

Yeah, to summarize my thoughts - the existing workflows are push_request and pull_request and are organized by what we should do for pushes vs. pull requests. But I think something like running tests we want to happen in both of those cases - on all prs and on at least pushes to main.

Aha thank you. Like this.

@rlinehan
Copy link
Contributor Author

@gautamomento my thinking was add ci.yml in this pr, and then in another can refactor the other two existing workflows to be more like what I described?

@gautamomento
Copy link
Contributor

@gautamomento my thinking was add ci.yml in this pr, and then in another can refactor the other two existing workflows to be more like what I described?

That sounds reasonable. Please add an issue here so we don't miss it.

@rlinehan
Copy link
Contributor Author

That sounds reasonable. Please add an issue here so we don't miss it.

Added #19

@rlinehan rlinehan merged commit 03f50e5 into main Jan 13, 2022
@rlinehan rlinehan deleted the rlinehan/integration-tests branch January 13, 2022 18:19
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.

3 participants