Skip to content

feat: add test for get_nfpm_arch() #4194

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 1 commit into from
Sep 17, 2021
Merged

feat: add test for get_nfpm_arch() #4194

merged 1 commit into from
Sep 17, 2021

Conversation

jsjoeio
Copy link
Contributor

@jsjoeio jsjoeio commented Sep 16, 2021

This adds tests for build-packages.sh, specifically

This PR introduces a new file called build-lib.sh which extracts the get_nfpm_arch() function out of build-packages.sh so that it can be unit tested. I did this because if you source build-packages.sh it runs the main function.

With this new approach, we can move individual functions related to the build process into build-lib.sh and unit test them.

The inspiration for this came from some recent rockiness with the 3.12.0 release and our interest in stabilizing the build and release pipeline.

image

Fixes N/A

Note: I would love if we could fully test all our scripts used in the build and release process. I also tossed around the idea of rewriting these scripts in JS/TS but that's a discussion for the future 😉

@codecov
Copy link

codecov bot commented Sep 16, 2021

Codecov Report

Merging #4194 (fbd1386) into main (0609a1b) will not change coverage.
The diff coverage is n/a.

❗ Current head fbd1386 differs from pull request most recent head 6a69248. Consider uploading reports for the commit 6a69248 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4194   +/-   ##
=======================================
  Coverage   64.22%   64.22%           
=======================================
  Files          36       36           
  Lines        1873     1873           
  Branches      379      379           
=======================================
  Hits         1203     1203           
  Misses        569      569           
  Partials      101      101           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0609a1b...6a69248. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Sep 16, 2021

✨ Coder.com for PR #4194 deployed! It will be updated on every commit.

@jsjoeio jsjoeio self-assigned this Sep 17, 2021
@jsjoeio jsjoeio added the testing Anything related to testing label Sep 17, 2021
@jsjoeio jsjoeio added this to the 3.12.1 milestone Sep 17, 2021
@jsjoeio jsjoeio changed the title feat: add test for build-packages.sh feat: add test for get_nfpm_arch() Sep 17, 2021
@jsjoeio jsjoeio force-pushed the jsjoeio-add-sh-test branch from 5a78300 to 6967c6f Compare September 17, 2021 19:21
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 17, 2021

I don't know why this is passing locally but failing in CI 🤔

image

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Sep 17, 2021

We think it's because the runner doesn't have jq installed. Going to refactor this function to a case statement instead

@jsjoeio jsjoeio force-pushed the jsjoeio-add-sh-test branch from fbd1386 to 24708b3 Compare September 17, 2021 22:11
@jsjoeio jsjoeio marked this pull request as ready for review September 17, 2021 22:18
@jsjoeio jsjoeio requested a review from a team as a code owner September 17, 2021 22:18
@jsjoeio jsjoeio force-pushed the jsjoeio-add-sh-test branch from 24708b3 to d06f6a8 Compare September 17, 2021 22:43
@jsjoeio jsjoeio enabled auto-merge September 17, 2021 22:44
@jsjoeio jsjoeio force-pushed the jsjoeio-add-sh-test branch from d06f6a8 to 6a69248 Compare September 17, 2021 23:07
@jsjoeio jsjoeio merged commit 4f3c8a5 into main Sep 17, 2021
@jsjoeio jsjoeio deleted the jsjoeio-add-sh-test branch September 17, 2021 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Anything related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants