Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add _test suffix #1395

Merged
merged 1 commit into from
May 11, 2022
Merged

Add _test suffix #1395

merged 1 commit into from
May 11, 2022

Conversation

helsaawy
Copy link
Contributor

Several test\cri-containerd files do not end with _test, which presents issues with the language server.
Additionally, if test\go.mod is removed, and test becomes part of the hcsshim module, then these files' dependencies would not be categorized as test dependencies.

Additionally, since these files contained testing helper functions, they are prepended with helper_ to better identify them.

Signed-off-by: Hamza El-Saawy hamzaelsaawy@microsoft.com

@helsaawy
Add _test suffix
d9d844f 
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
@jterry75
Copy link
Contributor

I think thats because this wasn't a go test binary as much as it was a binary that was used for testing originally.

LGTM

@helsaawy helsaawy marked this pull request as ready for review May 11, 2022 17:43
@helsaawy helsaawy requested a review from a team as a code owner May 11, 2022 17:43
kevpar
kevpar approved these changes May 11, 2022
View reviewed changes
Copy link
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

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

LGTM

@helsaawy helsaawy merged commit d4f0f15 into microsoft:master May 11, 2022
@helsaawy helsaawy deleted the he/testnames branch May 11, 2022 17:57
@helsaawy helsaawy restored the he/testnames branch May 11, 2022 17:58
@helsaawy helsaawy deleted the he/testnames branch May 11, 2022 17:58
anmaxvl added a commit that referenced this pull request Feb 7, 2023
@anmaxvl
Merged PR 6117553: Sync ADO to 671ec23
6342f80 
Sync ADO to 671ec23

Related work items: #1258, #1273, #1304, #1359, #1360, #1362, #1363, #1367, #1372, #1374, #1377, #1380, #1381, #1382, #1384, #1387, #1388, #1389, #1390, #1391, #1393, #1394, #1395, #1396, #1398, #1400, #1401, #1402, #1404, #1406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevpar kevpar kevpar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@helsaawy @jterry75 @kevpar