Skip to content

epoch: fix unit test when SOURCE_DATE_EPOCH is set#8202

Merged
AkihiroSuda merged 1 commit intocontainerd:mainfrom
samuelkarp:issue-8200
Mar 4, 2023
Merged

epoch: fix unit test when SOURCE_DATE_EPOCH is set#8202
AkihiroSuda merged 1 commit intocontainerd:mainfrom
samuelkarp:issue-8200

Conversation

@samuelkarp
Copy link
Copy Markdown
Member

@samuelkarp samuelkarp commented Mar 3, 2023

Fixes #8200

Before this change, the following invocations would cause the unit test to fail:

$ SOURCE_DATE_EPOCH="" go test -v ./pkg/epoch
=== RUN   TestSourceDateEpoch
    epoch_test.go:38: SOURCE_DATE_EPOCH is already set to "", unsetting
=== RUN   TestSourceDateEpoch/WithoutSourceDateEpoch
    epoch_test.go:44: 
        	Error Trace:	/home/samuelkarp/go/src/github.com/containerd/containerd/pkg/epoch/epoch_test.go:44
        	Error:      	Received unexpected error:
        	            	invalid SOURCE_DATE_EPOCH value "": strconv.ParseInt: parsing "": invalid syntax
        	Test:       	TestSourceDateEpoch/WithoutSourceDateEpoch
=== RUN   TestSourceDateEpoch/WithSourceDateEpoch
=== RUN   TestSourceDateEpoch/WithInvalidSourceDateEpoch
time="2023-03-03T14:54:13-08:00" level=warning msg="Invalid SOURCE_DATE_EPOCH" error="invalid SOURCE_DATE_EPOCH value \"foo\": strconv.ParseInt: parsing \"foo\": invalid syntax"
--- FAIL: TestSourceDateEpoch (0.00s)
    --- FAIL: TestSourceDateEpoch/WithoutSourceDateEpoch (0.00s)
    --- PASS: TestSourceDateEpoch/WithSourceDateEpoch (0.00s)
    --- PASS: TestSourceDateEpoch/WithInvalidSourceDateEpoch (0.00s)
FAIL
FAIL	github.com/containerd/containerd/pkg/epoch	0.004s
FAIL

$ SOURCE_DATE_EPOCH="asdfasdf" go test -v ./pkg/epoch
=== RUN   TestSourceDateEpoch
    epoch_test.go:38: SOURCE_DATE_EPOCH is already set to "asdfasdf", unsetting
=== RUN   TestSourceDateEpoch/WithoutSourceDateEpoch
    epoch_test.go:44: 
        	Error Trace:	/home/samuelkarp/go/src/github.com/containerd/containerd/pkg/epoch/epoch_test.go:44
        	Error:      	Received unexpected error:
        	            	invalid SOURCE_DATE_EPOCH value "": strconv.ParseInt: parsing "": invalid syntax
        	Test:       	TestSourceDateEpoch/WithoutSourceDateEpoch
=== RUN   TestSourceDateEpoch/WithSourceDateEpoch
=== RUN   TestSourceDateEpoch/WithInvalidSourceDateEpoch
time="2023-03-03T14:54:03-08:00" level=warning msg="Invalid SOURCE_DATE_EPOCH" error="invalid SOURCE_DATE_EPOCH value \"foo\": strconv.ParseInt: parsing \"foo\": invalid syntax"
--- FAIL: TestSourceDateEpoch (0.00s)
    --- FAIL: TestSourceDateEpoch/WithoutSourceDateEpoch (0.00s)
    --- PASS: TestSourceDateEpoch/WithSourceDateEpoch (0.00s)
    --- PASS: TestSourceDateEpoch/WithInvalidSourceDateEpoch (0.00s)
FAIL
FAIL	github.com/containerd/containerd/pkg/epoch	0.005s
FAIL

After this change, both scenarios pass.

@samuelkarp samuelkarp requested a review from AkihiroSuda March 3, 2023 22:56
@samuelkarp samuelkarp mentioned this pull request Mar 3, 2023
Closed
@samuelkarp
Copy link
Copy Markdown
Member Author

samuelkarp commented Mar 3, 2023

samuelkarp force-pushed the issue-8200 branch from c2b404c to 8ce3e4e

Added an explicit test case for an empty SOURCE_DATE_EPOCH

@ruiwen-zhao
Copy link
Copy Markdown
Member

ruiwen-zhao commented Mar 3, 2023

/lgtm

So I guess this change doesnt only fix the unit test, but also makes containerd handle the case where envvar SOURCE_DATE_EPOCH is set to "", which is the trigger of the ubuntu test failure in #8200

@samuelkarp
Copy link
Copy Markdown
Member Author

samuelkarp commented Mar 3, 2023

Correct, this fixes both the logic and the unit test. Happy to rephrase the commit message if you think that'd be helpful.

@samuelkarp
Copy link
Copy Markdown
Member Author

samuelkarp commented Mar 3, 2023

which is the trigger of the ubuntu test failure in #8200

The trigger is actually that it's set to any value (including but not limited to ""). The unit test used t.Setenv("SOURCE_DATE_EPOCH", "") to clear it, but that set the value to "" rather than unsetting. The package logic only handled the case where it was unset, not set to "".

@samuelkarp samuelkarp added this to the 1.7 milestone Mar 4, 2023
@AkihiroSuda AkihiroSuda merged commit 1e0e909 into containerd:main Mar 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mxpv mxpv mxpv approved these changes

@estesp estesp estesp approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.7

Development

Successfully merging this pull request may close these issues.

Test failure against 1.7.0-rc.1 release

5 participants

@samuelkarp @ruiwen-zhao @mxpv @estesp @AkihiroSuda