Skip to content

Conversation

@testwill
Copy link

io/ioutil deprecated etc.


.DS_Store

.idea/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not related. Please remove this change.

// uses interface information unlike golang github.com/docker/libkv/store/mock.(*Mock).WatchTree
// With GCCGO we need to remove interface information starting from pN<dd>.
re := regexp.MustCompile("\\.pN\\d+_")
re := regexp.MustCompile(`\.pN\d+_`)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not needed. This is just a quotes changes. While I agree that it improves redability, it has nothing to do in a MR that claims "code optimization". Please make a separate MR.

for i := 0; i < len(expected); i++ {
arr[i] = expected[i]
}
copy(arr, expected)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better:

// Clone expected into arr
arr := append(([]interface{})(nil), expected)


require.Equal(t, 1, len(tcl.errs))
assert.Regexp(t, regexp.MustCompile("(?s)FAIL: 0 out of 1 expectation\\(s\\) were met.*The code you are testing needs to make 1 more call\\(s\\).*"), tcl.errs[0])
assert.Regexp(t, regexp.MustCompile(`(?s)FAIL: 0 out of 1 expectation\(s\) were met.*The code you are testing needs to make 1 more call\(s\).*`), tcl.errs[0])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes quoting. This is an unrelated change.

assert.Regexp(t, regexp.MustCompile(`(?s)FAIL: 0 out of 1 expectation\(s\) were met.*The code you are testing needs to make 1 more call\(s\).*`), tcl.errs[0])
require.Equal(t, 2, len(tcl.logs))
assert.Regexp(t, regexp.MustCompile("(?s)FAIL:\tGetTime\\(int\\).*"), tcl.logs[0])
assert.Regexp(t, regexp.MustCompile(`(?s)FAIL:\tGetTime\(int\).*`), tcl.logs[0])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes quoting. This is an unrelated change.

func TestAfterTotalWaitTimeWhileExecution(t *testing.T) {
waitDuration := 1
total, waitMs := 5, time.Millisecond*time.Duration(waitDuration)
total, wait := 5, time.Millisecond*time.Duration(waitDuration)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes a variable name. This is an unrelated change. Remove it.

os.Stdout.Close()
os.Stdout = sc.oldStdout
bytes, err := ioutil.ReadAll(sc.readPipe)
bytes, err := io.ReadAll(sc.readPipe)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change requires Go 1.16. testify is used in a wide range of project. This is not a good thing to force users to upgrade Go while it is not necessary (io/ioutil will stay forever).

@dolmen
Copy link
Collaborator

dolmen commented Jul 4, 2023

This PR bundles too many unrelated changes with a misleading title. I recommend @testwill to close it and submit more targetted changes and take more care in the description of the PR.

@dolmen dolmen closed this Jul 5, 2023
@dolmen dolmen added rejected/duplicate internal/refactor Refactor internals with no external visible changes labels Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal/refactor Refactor internals with no external visible changes rejected/duplicate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants