Revise testing policy with labels #57

hlandau · 2022-10-20T15:29:54Z

First draft. Let me know your thoughts. I've tried to avoid overengineering this for now.

policies/testing.md

mattcaswell

LGTM

policies/testing.md

hlandau · 2022-10-21T09:56:00Z

Updated.

beldmit · 2022-10-21T10:00:08Z

policies/testing.md

+- `tests: deferred`, where the tests are required by the testing policy
+  but it is intended to add these tests in a subsequent PR. The label should
+  be removed (for example from a closed PR) once subsequent tests have
+  been merged.


How do we deal with situations where the PR author considers that the current test suite is sufficient?

I think this would come under tests: present.

Is this fail to merge without one of these labels being set, going to display a relevant error?

How would you want to do this. It would require the GHE hooks to look at the PRs in the merged commits which I do not think we want.

I think this would come under tests: present.

It may also be "tests: exempted". For example a refactor is specifically exempted.

I agree with @t8m re automatic checks, because of all the churn (and because github has a 5s time limit on pre-receive hooks, where those checks need to be placed)

t8m

I think this is a good start for now.

slontis · 2022-10-21T23:54:11Z

policies/testing.md

+labels must be applied:
+
+- `tests: present`, to be added where suitable tests are included in the same PR.
+- `hold: tests needed`, where the testing policy requires tests but they are not


Do some of these tests labels need explicit statements by each approver (I agree with the label X).
(Like we do for trivial)

I think it should be up to the approver to disagree with the choice.

policies/testing.md

hlandau · 2022-11-07T10:11:16Z

It's been 18 days, so I'm now raising the vote on this. cc @openssl/otc

topic: Approve the proposed test labelling policy (technical-policies#57)
Proposed by Hugo Landau
Public: yes
opened: 2022-11-07
closed: 
accepted:  yes/no  (for: X, against: Y, abstained: Z, not voted: T)

  Dmitry     [  ]
  Matt       [  ]
  Pauli      [  ]
  Tim        [  ]
  Hugo       [+1]
  Richard    [  ]
  Shane      [  ]
  Tomas      [  ]
  Kurt       [  ]
  Matthias   [  ]
  Nicola     [  ]

t8m · 2022-11-07T10:27:07Z

Vote: [+1]

t-j-h · 2022-11-07T10:39:55Z

Vote [0]

mattcaswell · 2022-11-07T10:55:58Z

Vote: [+1]

kroeckx · 2022-11-07T11:00:42Z

Voting +1

paulidale · 2022-11-07T21:21:07Z

Vote: [+1]

slontis · 2022-11-07T21:24:49Z

Vote +1

…

________________________________ From: Pauli ***@***.***> Sent: Tuesday, November 8, 2022 7:21 AM To: openssl/technical-policies ***@***.***> Cc: Shane Lontis ***@***.***>; Comment ***@***.***> Subject: [External] : Re: [openssl/technical-policies] Revise testing policy with labels (PR #57) Vote: [+1] — Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/openssl/technical-policies/pull/57*issuecomment-1306209082__;Iw!!ACWV5N9M2RV99hQ!JZEsOB6_fFLxbduj79hUgn6xvEQpeLd9CKR7TSI9NOcaTCsjeAerbHftF6dsRyVbN2KIiElZko0ybfPodKsHAqgFBQ$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AJUSG6SEU35J2GIH7GI6K63WHFXE7ANCNFSM6AAAAAARKI3IBU__;!!ACWV5N9M2RV99hQ!JZEsOB6_fFLxbduj79hUgn6xvEQpeLd9CKR7TSI9NOcaTCsjeAerbHftF6dsRyVbN2KIiElZko0ybfPodKtlnDjDLw$>. You are receiving this because you commented.Message ID: ***@***.***>

beldmit · 2022-11-07T21:34:15Z

Vote: [+1]

mspncp · 2022-11-07T23:39:34Z

Vote: [+1]

mspncp · 2022-11-07T23:50:35Z

(Side Note: @hlandau it would be nice if you could follow the current coloring practice for the labels, i.e., reuse the existing red color for the hold: tests needed label (the hex value appears and can be copy&pasted when you 'edit' one of the existing labels) and choose a new, easily distinguishable color of your liking for the new tests: * labels.)

hlandau · 2022-11-08T07:49:10Z

(Side Note: @hlandau it would be nice if you could follow the current coloring practice for the labels, i.e., reuse the existing red color for the hold: tests needed label (the hex value appears and can be copy&pasted when you 'edit' one of the existing labels) and choose a new, easily distinguishable color of your liking for the new tests: * labels.)

Agreed, this is my plan: red for the hold, something visually distinct for the other test labels.

hlandau · 2022-11-10T09:02:24Z

Since the outcome of the vote is now a foregone conclusion, this vote can now be closed.

topic: Approve the proposed test labelling policy (technical-policies#57)
Proposed by Hugo Landau
Public: yes
opened: 2022-11-07
closed: 2022-11-10
accepted:  yes  (for: 8, against: 0, abstained: 1, not voted: 2)

  Dmitry     [+1]
  Matt       [+1]
  Pauli      [+1]
  Tim        [ 0]
  Hugo       [+1]
  Richard    [  ]
  Shane      [+1]
  Tomas      [+1]
  Kurt       [+1]
  Matthias   [+1]
  Nicola     [  ]

romen · 2022-11-10T13:41:35Z

Vote: [+0]

t8m · 2022-11-10T14:32:58Z

I've immediately realized that we could avoid some labeling pollution by not requiring the test: exempted label on PRs that are just triaged: documentation or triaged: cleanup or triaged: design or maybe even triaged: refactor.

hlandau added the policy change A change to a policy is being proposed label Oct 20, 2022

hlandau requested review from t8m and mattcaswell October 20, 2022 15:29

hlandau self-assigned this Oct 20, 2022

paulidale reviewed Oct 20, 2022

View reviewed changes

policies/testing.md Outdated Show resolved Hide resolved

levitte reviewed Oct 21, 2022

View reviewed changes

policies/testing.md Show resolved Hide resolved

levitte reviewed Oct 21, 2022

View reviewed changes

policies/testing.md Show resolved Hide resolved

levitte reviewed Oct 21, 2022

View reviewed changes

policies/testing.md Outdated Show resolved Hide resolved

mattcaswell reviewed Oct 21, 2022

View reviewed changes

policies/testing.md Show resolved Hide resolved

policies/testing.md Show resolved Hide resolved

beldmit reviewed Oct 21, 2022

View reviewed changes

levitte approved these changes Oct 21, 2022

View reviewed changes

t8m approved these changes Oct 21, 2022

View reviewed changes

slontis reviewed Oct 21, 2022

View reviewed changes

paulidale approved these changes Oct 23, 2022

View reviewed changes

mattcaswell approved these changes Oct 24, 2022

View reviewed changes

paulidale reviewed Oct 25, 2022

View reviewed changes

policies/testing.md Show resolved Hide resolved

t8m added the discussed The issue/pr was discussed by the OTC label Oct 25, 2022

t8m added the ready to vote The policy change proposal is ready to be voted on by the OTC label Nov 7, 2022

hlandau added 2 commits November 10, 2022 09:09

Revise testing policy with labels

a7a5c8b

Add vote for new test labels policy

95becf1

hlandau force-pushed the new-testing-policy branch from 1b5713e to 95becf1 Compare November 10, 2022 09:10

hlandau merged commit 95becf1 into openssl:master Nov 10, 2022

hlandau mentioned this pull request Nov 10, 2022

Creation of testing labels openssl/openssl#19640

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revise testing policy with labels #57

Revise testing policy with labels #57

hlandau commented Oct 20, 2022

mattcaswell left a comment

hlandau commented Oct 21, 2022

beldmit Oct 21, 2022

hlandau Oct 21, 2022

slontis Oct 21, 2022 •

edited

Loading

t8m Oct 22, 2022

mattcaswell Oct 24, 2022

levitte Oct 25, 2022

t8m left a comment

slontis Oct 21, 2022

paulidale Oct 23, 2022

hlandau commented Nov 7, 2022

t8m commented Nov 7, 2022

t-j-h commented Nov 7, 2022

mattcaswell commented Nov 7, 2022

kroeckx commented Nov 7, 2022

paulidale commented Nov 7, 2022

slontis commented Nov 7, 2022 via email

beldmit commented Nov 7, 2022

mspncp commented Nov 7, 2022

mspncp commented Nov 7, 2022

hlandau commented Nov 8, 2022

hlandau commented Nov 10, 2022

romen commented Nov 10, 2022

t8m commented Nov 10, 2022

Revise testing policy with labels #57

Revise testing policy with labels #57

Conversation

hlandau commented Oct 20, 2022

mattcaswell left a comment

Choose a reason for hiding this comment

hlandau commented Oct 21, 2022

beldmit Oct 21, 2022

Choose a reason for hiding this comment

hlandau Oct 21, 2022

Choose a reason for hiding this comment

slontis Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

t8m Oct 22, 2022

Choose a reason for hiding this comment

mattcaswell Oct 24, 2022

Choose a reason for hiding this comment

levitte Oct 25, 2022

Choose a reason for hiding this comment

t8m left a comment

Choose a reason for hiding this comment

slontis Oct 21, 2022

Choose a reason for hiding this comment

paulidale Oct 23, 2022

Choose a reason for hiding this comment

hlandau commented Nov 7, 2022

t8m commented Nov 7, 2022

t-j-h commented Nov 7, 2022

mattcaswell commented Nov 7, 2022

kroeckx commented Nov 7, 2022

paulidale commented Nov 7, 2022

slontis commented Nov 7, 2022 via email

beldmit commented Nov 7, 2022

mspncp commented Nov 7, 2022

mspncp commented Nov 7, 2022

hlandau commented Nov 8, 2022

hlandau commented Nov 10, 2022

romen commented Nov 10, 2022

t8m commented Nov 10, 2022

slontis Oct 21, 2022 •

edited

Loading