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.

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

Update testing guideline & template #1791

Merged
merged 13 commits into from
Oct 3, 2022
Merged

Conversation

kevindiu
Copy link
Contributor

@kevindiu kevindiu commented Sep 22, 2022

Signed-off-by: kevindiu kevindiujp@gmail.com

Description:

Grammar checked

This PR updates testing guideline to include test changes on PR#1734 which includes the following changes:

Testing template:

  • template changes for test helper

Documentation:

Things to add:

  • Usage of context in test case
    • Use context.WithCancel(context.Background()) if target test function accept context as argument
  • Different usage of goleak.VerifyMain() and goleak.VerifyNone()
    • verifyMain verify if any goroutine leaks after all the test case is finished
    • verifyNone verify each test case
    • Use verifyMain when something depends on context or something keepalive even after test case is finished
  • Use of parallel test
    • advantages of using parallel test, and challenges
  • Use initialization function (e.g. New()) to instantiate target object
    • Use functional option for test
    • reason: Use of default values from initialization function
  • t.Helper() usage

Things to change/update:

Related Issue:

How Has This Been Tested?:

Environment:

  • Go Version: 1.19.1
  • Docker Version: 20.10.8
  • Kubernetes Version: 1.22.0
  • NGT Version: 1.14.7

Types of changes:

  • Bug fix [type/bug]
  • New feature [type/feature]
  • Add tests [type/test]
  • Security related changes [type/security]
  • Add documents [type/documentation]
  • Refactoring [type/refactoring]
  • Update dependencies [type/dependency]
  • Update benchmarks and performances [type/bench]
  • Update CI [type/ci]

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Checklist:

  • I have read the CONTRIBUTING document.
  • I have checked open Pull Requests for the similar feature or fixes?
  • I have added tests and benchmarks to cover my changes.
  • I have ensured all new and existing tests passed.
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly.

Signed-off-by: kevindiu <kevindiujp@gmail.com>
@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - replace the PR body by changelog details
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • /rebase - rebase main
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

LanguageTool

docs/contributing/coding-style.md|1153 col 8| This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: For
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING
docs/contributing/coding-style.md|1153 col 33| Unpaired symbol: ‘}’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1155 col 17| Unpaired symbol: ‘)’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1155 col 49| Unpaired symbol: ‘}’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1159 col 19| Check that the noun ‘initialization’ after the pronoun ‘we’ is correct. It’s possible that you may need to switch to a possessive pronoun, or use another part of speech. (PRP_VB[1])
Rule: https://community.languagetool.org/rule/show/PRP_VB?lang=en-US&subId=1
Category: GRAMMAR
docs/contributing/coding-style.md|1177 col 44| Unpaired symbol: ‘}’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1192 col 8| This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: For
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING
docs/contributing/coding-style.md|1192 col 33| Unpaired symbol: ‘}’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1194 col 17| Unpaired symbol: ‘)’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1194 col 49| Unpaired symbol: ‘}’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1208 col 16| This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: S
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING
docs/contributing/coding-style.md|1228 col 20| This word is normally spelled as one. (EN_COMPOUNDS)
Suggestions: subtests
URL: https://languagetool.org/insights/post/hyphen/
Rule: https://community.languagetool.org/rule/show/EN_COMPOUNDS?lang=en-US
Category: MISC
docs/contributing/coding-style.md|1233 col 43| Unpaired symbol: ‘}’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1235 col 21| Unpaired symbol: ‘}’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1239 col 1| This sentence does not start with an uppercase letter. (UPPERCASE_SENTENCE_START)
Suggestions: For
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING
docs/contributing/coding-style.md|1239 col 26| Unpaired symbol: ‘}’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1241 col 7| Unpaired symbol: ‘)’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1241 col 39| Unpaired symbol: ‘}’ seems to be missing (EN_UNPAIRED_BRACKETS)
Rule: https://community.languagetool.org/rule/show/EN_UNPAIRED_BRACKETS?lang=en-US
Category: PUNCTUATION
docs/contributing/coding-style.md|1242 col 33| This word is normally spelled as one. (EN_COMPOUNDS)
Suggestions: subtests
URL: https://languagetool.org/insights/post/hyphen/
Rule: https://community.languagetool.org/rule/show/EN_COMPOUNDS?lang=en-US
Category: MISC

docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
Signed-off-by: kevindiu <kevindiujp@gmail.com>
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 22, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 85d3652
Status: ✅  Deploy successful!
Preview URL: https://1e00458b.vald.pages.dev
Branch Preview URL: https://documentation-update-test-gu.vald.pages.dev

View logs

docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
Signed-off-by: kevindiu <kevindiujp@gmail.com>
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
@kevindiu kevindiu changed the title [WIP] update testing guideline Update testing guideline Sep 22, 2022
@kevindiu kevindiu marked this pull request as ready for review September 22, 2022 02:50
@kpango kpango requested a review from hlts2 September 22, 2022 04:45
kevindiu and others added 2 commits September 26, 2022 11:47
Co-authored-by: Hiroto Funakoshi <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: kevindiu <kevindiujp@gmail.com>
hlts2
hlts2 previously approved these changes Sep 26, 2022
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM

docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
Signed-off-by: kevindiu <kevindiujp@gmail.com>
Signed-off-by: kevindiu <kevindiujp@gmail.com>
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
docs/contributing/coding-style.md Show resolved Hide resolved
@kevindiu kevindiu requested a review from hlts2 September 28, 2022 01:17
@kevindiu kevindiu changed the title Update testing guideline Update testing guideline & template Sep 28, 2022
hlts2
hlts2 previously approved these changes Sep 28, 2022
docs/contributing/coding-style.md Outdated Show resolved Hide resolved
Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>
@kpango kpango merged commit c428663 into main Oct 3, 2022
@kpango kpango deleted the documentation/update-test-guideline branch October 3, 2022 08:20
@kevindiu kevindiu mentioned this pull request Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants