feat(library): add Duration() function to resources

[jbrockopp]

Currently, we have logic to calculate the duration (amount of time a resource was running) for various resources.

The following resources have this functionality:

• build
• service
• step

This change introduces a Duration() method on each of these resources that will replace the above linked functions:

types/library/build.go

Lines 51 to 70 in 16db0db

	// Duration calculates and returns the total amount of
	// time the build ran for in a human-readable format.
	func (b *Build) Duration() string {
	// check if the build doesn't have a started or finished timestamp
	if b.GetStarted() == 0 || b.GetFinished() == 0 {
	// return zero value for time.Duration (0s)
	return new(time.Duration).String()
	}
	// capture finished unix timestamp from the build
	finished := time.Unix(b.GetFinished(), 0)
	// capture started unix timestamp from the build
	started := time.Unix(b.GetStarted(), 0)
	// calculate the duration by subtracting the build
	// started time from the build finished time
	duration := finished.Sub(started)
	// return duration in a human-readable form
	return duration.String()
	}

types/library/service.go

Lines 36 to 55 in 16db0db

	// Duration calculates and returns the total amount of
	// time the service ran for in a human-readable format.
	func (s *Service) Duration() string {
	// check if the service doesn't have a started or finished timestamp
	if s.GetStarted() == 0 || s.GetFinished() == 0 {
	// return zero value for time.Duration (0s)
	return new(time.Duration).String()
	}
	// capture finished unix timestamp from the service
	finished := time.Unix(s.GetFinished(), 0)
	// capture started unix timestamp from the service
	started := time.Unix(s.GetStarted(), 0)
	// calculate the duration by subtracting the service
	// started time from the service finished time
	duration := finished.Sub(started)
	// return duration in a human-readable form
	return duration.String()
	}

types/library/step.go

Lines 38 to 57 in 16db0db

	// Duration calculates and returns the total amount of
	// time the step ran for in a human-readable format.
	func (s *Step) Duration() string {
	// check if the service doesn't have a started or finished timestamp
	if s.GetStarted() == 0 || s.GetFinished() == 0 {
	// return zero value for time.Duration (0s)
	return new(time.Duration).String()
	}
	// capture finished unix timestamp from the step
	finished := time.Unix(s.GetFinished(), 0)
	// capture started unix timestamp from the step
	started := time.Unix(s.GetStarted(), 0)
	// calculate the duration by subtracting the step
	// started time from the step finished time
	duration := finished.Sub(started)
	// return duration in a human-readable form
	return duration.String()
	}

[codecov]

Codecov Report

Merging #222 (b3cbd9e) into master (34d807f) will increase coverage by 0.27%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #222      +/-   ##
==========================================
+ Coverage   96.74%   97.01%   +0.27%     
==========================================
  Files          53       53              
  Lines        5645     5690      +45     
==========================================
+ Hits         5461     5520      +59     
+ Misses        137      125      -12     
+ Partials       47       45       -2     

Impacted Files	Coverage Δ
library/build.go	100.00% <100.00%> (ø)
library/service.go	85.98% <100.00%> (+2.97%)	⬆️
library/step.go	100.00% <100.00%> (+1.98%)	⬆️

[ecrupper]

Looks good 👍 , one small typo that I spotted.

I think this is a lot cleaner than how it was previously set up with the duration helper functions. Thanks!

[wass3r]

an afterthought, should a build in progress (ie. started is set but finished is not set yet) return 0s for the duration? or the time since started?

[jbrockopp]

@wass3r initially, I thought the same thing and actually had written code to accomplish that 👍

However, when I started writing tests, I stumbled upon a reason why it may not be great for that to be the case 😅

The started timestamp we use in our test objects is 1563474078:

types/library/build_test.go

Line 810 in a10f36c

b.SetStarted(1563474078)

types/library/service_test.go

Line 356 in a10f36c

s.SetStarted(1563474078)

types/library/step_test.go

Line 449 in a10f36c

s.SetStarted(1563474078)

That UNIX timestamp actually refers to a time on Thursday, July 18, 2019 👍

After removing the finished timestamp from a test object, the Duration() function returned something like 2y5m10d.

In most scenarios, the functionality you described for durations would work 😃

The typical case for a resource to have a started timestamp but not a finished timestamp is due to it still running.

However, it's possible for a resource to get stuck/hung meaning it never gets a finished timestamp.

I decided to use 0s if the started or finished timestamp aren't set to help prevent misleading information.

[wass3r]

Isn't it still misleading though? A running build (not hung) will show its duration as 0s if the callee is to use the returned value as is.

I agree that hung builds are more of an outlier than running builds, so I think it's ok if the duration says something abnormally large.

[jbrockopp]

Isn't it still misleading though?

Yes, it is misleading either way 🤷

Would you rather have these functions return ... like we do in the CLI?

I agree that hung builds are more of an outlier than running builds, ...

Agreed 👍

so I think it's ok if the duration says something abnormally large.

Unfortunately, I don't agree 😞

[wass3r]

I guess we disagree.

I find it much more bothersome for a healthy running build to say it's been running for 0s (when it hasn't) than a rare hung build to tell me it has been running for 6 months. If used in the CLI (for example), I would not use the value as is. I think folks would prefer either the current running time, or something indeterminate like ....

[jbrockopp]

Would you like me to open up a PR that changes the behavior to ... then?

The only downside I see to that is ... isn't actually a valid duration 😞

If someone were to use the Duration() functions with time.ParseDuration() it would always return an error:

https://go.dev/play/p/_sIxzHTyc9n

On a positive note, the user would still get 0s as the return value since the default for time.Duration{} is 0s.