Unit tests #94
Merged
Unit tests #94
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
google-cla
bot
added
the
cla: yes
|
I'm +1 on GitHub Actions; the lack of a clear pass/fail indicator (e.g. on this build) hurts my eyes. 😬 |
Ah that's because it wasn't supposed to run on this PR because we didn't edit the website or api directories! It's a little annoying that it shows up with a grey symbol. |
|
Our current decision on CI tools is to use GitHub Actions for static analysis. Unit tests fall into a gray area. I'm in favor of inverting the decision a bit: using Cloud Build only when managing Google Cloud resources is required. Given this is already implemented with Cloud Build, is there an advantage to keeping it in GCB we should be aware of as a trade-off? |
|
I think the only advantage is having more Cloud Build artifacts and use-cases covered. Maybe the right thing to do is to write a friction log highlighting the differences between Cloud Build and GitHub Actions for unit testing? |
|
I propose merging this and then I'll re-implement it in GitHub Actions and write a friction log covering both pathways, covering:
|
|
That sounds good, and will also make for a richer decision record. My approach to 1P vs. 3rd Party includes a couple checks:
Using our work here to point out needed product improvements is one of our goals. Could you file follow-up issues for
|
grayside
approved these changes
Jul 12, 2021
engelke
added a commit
that referenced
this pull request
Jul 16, 2021
* Api cleanup (#92) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * Unit tests (#94) * Unit tests * Adding directory * Using alpine image and installing pytest * Using slim image instead * Adding descriptive ID's for the steps * productivity(terraform): correct terraform violations via PR suggestions (#98) * productivity(terraform): correct terraform violations via PR suggestions (fixes #50) * Switch to secrets.GITHUB_TOKEN Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> * Update decisions.md (#108) New decision record * Terraform and setup.sh (#93) Initial terraform setup. Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com>
engelke
added a commit
that referenced
this pull request
Jul 16, 2021
* OpenAPI spec and generated Python library * More operations in spec * Api cleanup (#92) (#101) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * OpenAPI complete, not finished * Client library generated * Update branch to match main (#113) * Api cleanup (#92) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * Unit tests (#94) * Unit tests * Adding directory * Using alpine image and installing pytest * Using slim image instead * Adding descriptive ID's for the steps * productivity(terraform): correct terraform violations via PR suggestions (#98) * productivity(terraform): correct terraform violations via PR suggestions (fixes #50) * Switch to secrets.GITHUB_TOKEN Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> * Update decisions.md (#108) New decision record * Terraform and setup.sh (#93) Initial terraform setup. Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com>
grayside
pushed a commit
that referenced
this pull request
Jul 22, 2021
* Unit tests * Adding directory * Using alpine image and installing pytest * Using slim image instead * Adding descriptive ID's for the steps
grayside
added a commit
that referenced
this pull request
Jul 22, 2021
* OpenAPI spec and generated Python library * More operations in spec * Api cleanup (#92) (#101) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * OpenAPI complete, not finished * Client library generated * Update branch to match main (#113) * Api cleanup (#92) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * Unit tests (#94) * Unit tests * Adding directory * Using alpine image and installing pytest * Using slim image instead * Adding descriptive ID's for the steps * productivity(terraform): correct terraform violations via PR suggestions (#98) * productivity(terraform): correct terraform violations via PR suggestions (fixes #50) * Switch to secrets.GITHUB_TOKEN Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> * Update decisions.md (#108) New decision record * Terraform and setup.sh (#93) Initial terraform setup. Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Simple unit test execution. Created two triggers - one for the
websitedirectory and one forcontent-api.Alternatively, should this be moved to GitHub Actions? The logs are not easily viewable from GitHub.