-
Notifications
You must be signed in to change notification settings - Fork 136
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
fix: .gitignore .func #1728
fix: .gitignore .func #1728
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1728 +/- ##
===========================================
+ Coverage 49.66% 59.76% +10.10%
===========================================
Files 82 94 +12
Lines 11474 12343 +869
===========================================
+ Hits 5698 7377 +1679
+ Misses 5281 4285 -996
- Partials 495 681 +186
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
7b0e672
to
27d36f3
Compare
@lkingland the linter fails with
|
// TestClient_BuiltDetects ensures that the client's Built command detects | ||
// filesystem changes as indicating the function is no longer Built (aka stale) | ||
// This includes modifying timestamps, removing or adding files. | ||
func TestClient_BuiltDetects(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test was just moved in its entirety to be with the other functions tests, since the Built
logic is now a method on Function rather than a package static function.
@@ -394,19 +408,46 @@ func (f Function) Write() (err error) { | |||
// | |||
// The runtime data directory .func is created in the function root if | |||
// necessary. | |||
func (f Function) Stamp() (err error) { | |||
func (f Function) Stamp(oo ...stampOption) (err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Journaling is very much an optional, debugging-focused feature, so I used the option pattern here to retain the default API as the happy-path
8bda323
to
ad45e9c
Compare
08865da
to
9e1f52d
Compare
@@ -960,6 +966,178 @@ func (c *Client) Push(ctx context.Context, f Function) (Function, error) { | |||
return f, nil | |||
} | |||
|
|||
// ensureRunDataDir creates a .func directory at the given path, and | |||
// registers it as ignored in a .gitignore file. | |||
func ensureRunDataDir(root string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions (ensureRunDataDir
, fingerprint
, ...) support the functionality of the Client methods, so they were moved from function.go. Keeping Function as close to a data object as possible, with Client being the actor. The only one appreciably modified was this one, ensureRunDataDir
.
@@ -51,6 +52,138 @@ func TestClient_New(t *testing.T) { | |||
} | |||
} | |||
|
|||
// TestClient_New_RunData ensures that the .func runtime directory is | |||
// correctly created. | |||
func TestClient_New_RunDataDir(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the new test
- The .gitignore file was always relative to process' current working directory. Now correctly uses the function root - The .gitignore file was always being updated, causing its modification timestamp to be updated multiple times throughout the executaion of any client commands. - Adds the ability to override this behavior by commenting out the line in the .gitignore. - Adds the ability to request that stamping create an ongoing journal via a build log file with timestamp prefix (for debugging)
128fc0e
to
014ef00
Compare
Running |
Although this might not be caused by this PR. |
Thanks for taking a look. Yes caching is something that is still not happening with complete reliability. This PR gets it working in a few more scenarios and makes it more auditable so we can debug the others more easily. In particular it helps enable the changes to |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lkingland, matejvasek The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/kind bug
/kind enhancement