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

Multiple exempt-pr-label where labels have spaces #98

Closed
brianeclow opened this issue Jun 24, 2020 · 26 comments · Fixed by #199
Closed

Multiple exempt-pr-label where labels have spaces #98

brianeclow opened this issue Jun 24, 2020 · 26 comments · Fixed by #199
Assignees
Labels
bug Something isn't working

Comments

@brianeclow
Copy link

I would like to have two labels to exclude this action. From the examples I could find, a comma separated quoted string appears to be the correct way. However, most of our labels have spaces, unlike the examples. The following represents our configuration, however, it still triggered the action and closed a PR with both labels.

  • Did I miss some additional configuration?
  • Could a better example be provided?
---
name: "Close Stale Pull Requests"
on:
  schedule:
  - cron: "0 0 * * *"

jobs:
  stale:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/stale@v3
      with:
        repo-token: ${{ secrets.GITHUB_TOKEN }}
        stale-pr-label: 'stale'
        exempt-pr-label: 'work in progress,Do Not Merge'
@arirawr
Copy link

arirawr commented Jun 25, 2020

I have also been having issues with our labels, which have spaces. A better example would be great!

@brianeclow
Copy link
Author

Any word on this?
This has become a point of contention.

@stephenbawks
Copy link

Also came here looking to see how to handle Labels with white space and special characters. We typically have Labels that are descriptive and easy to read.

Examples:

Priority: Medium
Status: Work In Progress
Type: Bug

@bacongobbler
Copy link

Hmm... I was unable to reproduce this issue using the unit tests provided. I'm not sure if this is isolated for PR labels or not. But for reference, there's the unit test that covers this case:

test('exempt issue labels will not be marked stale (multi issue label with spaces)', async () => {
const TestIssueList: Issue[] = [
generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, ['Cool'])
];
const opts = {...DefaultProcessorOptions};
opts.exemptIssueLabels = 'Exempt, Cool, None';
const processor = new IssueProcessor(
opts,
async p => (p == 1 ? TestIssueList : []),
async (num, dt) => [],
async (issue, label) => new Date().toDateString()
);
// process our fake issue list
await processor.processIssues(1);
expect(processor.staleIssues.length).toEqual(0);
expect(processor.closedIssues.length).toEqual(0);
});

I modified the test case to add a label called Keep Open, and found that the test results were OK:

~/code/stale ><> git diff
diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts
index ed6d67f..f5a2939 100644
--- a/__tests__/main.test.ts
+++ b/__tests__/main.test.ts
@@ -495,11 +495,11 @@ test('exempt issue labels will not be marked stale', async () => {
 
 test('exempt issue labels will not be marked stale (multi issue label with spaces)', async () => {
   const TestIssueList: Issue[] = [
-    generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, ['Cool'])
+    generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, ['Keep Open'])
   ];
 
   const opts = {...DefaultProcessorOptions};
-  opts.exemptIssueLabels = 'Exempt, Cool, None';
+  opts.exemptIssueLabels = 'Exempt, Cool, None,Keep Open';
 
   const processor = new IssueProcessor(
     opts,
~/code/stale ><> npm test
Test Suites: 1 passed, 1 total
Tests:       31 passed, 31 total
Snapshots:   0 total
Time:        0.515s, estimated 1s
Ran all test suites.

So either this issue has been fixed, or the unit tests are presenting a false positive.

@stephenbawks
Copy link

stephenbawks commented Aug 19, 2020

I managed to make this work by just using the ASCII code for a space.

stale-issue-label: 'Status%3A%20Abandoned'

Translated my label which is 'Status: Abadonded'

@brianeclow
Copy link
Author

The specific failing case is multiple labels with spaces, a test with this label set would be more accurate.

generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, ['Keep Open', 'Magic Door'])
opts.exemptIssueLabels = 'Exempt, Cool, None, Keep Open, Magic Door';

@bacongobbler
Copy link

I tried that as well. The tests still passed.

><> git diff
diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts
index ed6d67f..5a2481f 100644
--- a/__tests__/main.test.ts
+++ b/__tests__/main.test.ts
@@ -495,11 +495,11 @@ test('exempt issue labels will not be marked stale', async () => {
 
 test('exempt issue labels will not be marked stale (multi issue label with spaces)', async () => {
   const TestIssueList: Issue[] = [
-    generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, ['Cool'])
+    generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, ['Keep Open', 'Magic Door'])
   ];
 
   const opts = {...DefaultProcessorOptions};
-  opts.exemptIssueLabels = 'Exempt, Cool, None';
+  opts.exemptIssueLabels = 'Exempt, Cool, None, Keep Open, Magic Door';
 
   const processor = new IssueProcessor(
     opts,
><> npm test
...
Test Suites: 1 passed, 1 total
Tests:       31 passed, 31 total
Snapshots:   0 total
Time:        0.518s, estimated 2s
Ran all test suites.

@stephenbawks may be onto something, though. It's possible that the labels are being passed along to the github action in their URL-encoded form.

@brianeclow
Copy link
Author

Yes, that has potential, however, it fails the ease of use category. 😆

This also begs the question, why would one label with spaces be ok, while two labels with spaces not be ok? My original setup only, which I have reverted to using, has work in progress as the only exempt label. And the stale action adheres to the ignore.

@brianeclow
Copy link
Author

I stand corrected, it is now failing with a single label. Adding the encoding, as a stop gap.

@hross hross added the bug Something isn't working label Aug 28, 2020
@hross hross self-assigned this Aug 28, 2020
martin-helmich added a commit to mittwald/kube-httpcache that referenced this issue Sep 15, 2020
Exempt issue names apparently need to be specified with URL encoding (see actions/stale#98).
martin-helmich added a commit to mittwald/kube-httpcache that referenced this issue Sep 15, 2020
Exempt issue names apparently need to be specified with URL encoding (see actions/stale#98).
@brianeclow
Copy link
Author

I added %20 in place of the spaces a month ago, and it is still marking PRs with exception labels that contain spaces as stale. @stephenbawks were you able to have the ASCII replacement work for the exempt-pr-label option?

@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the Stale label Oct 18, 2020
@C0ZEN
Copy link
Contributor

C0ZEN commented Oct 18, 2020

Commented to avoid stale ;)

@brianeclow
Copy link
Author

I was on vacation when my reminder went off for this. 😆
Is there any update?

@C0ZEN
Copy link
Contributor

C0ZEN commented Oct 18, 2020

@hross you assigned this one to you but I wish to help on this one.
If this is ok with you I will provide a PR tomorrow.
Also could you give me the right to push?

@github-actions github-actions bot removed the Stale label Oct 19, 2020
C0ZEN added a commit to C0ZEN/stale that referenced this issue Oct 19, 2020
at first I thought that the parseCommaSeparatedString method was causing the issue so I move it to a dedicated file to test it since it was private
also because I think that this repo could have more clean code and code splitting
anyway this was not the root of the actions#98 issue :/
@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the Stale label Nov 19, 2020
@brianeclow
Copy link
Author

Any update?

@github-actions github-actions bot removed the Stale label Nov 20, 2020
hross pushed a commit that referenced this issue Nov 20, 2020
* chore(git): ignore .idea folder to avoid adding WebStorm files

* test(jest): find all spec files as well

* refactor(labels): create a dedicated function to parse the labels

at first I thought that the parseCommaSeparatedString method was causing the issue so I move it to a dedicated file to test it since it was private
also because I think that this repo could have more clean code and code splitting
anyway this was not the root of the #98 issue :/

* fix(label): allow to use spaces inside the labels

* docs(isLabeled): add JSDoc

* chore(npm): add lint:fix script
@C0ZEN
Copy link
Contributor

C0ZEN commented Nov 20, 2020

@brianeclow might want to check if anything is alright.

@brianeclow
Copy link
Author

Nope, still broken... And now, with more fun.
The file:

name: "Close Stale Pull Requests"
on:
  schedule:
  - cron: "0 0 * * *"
  workflow_dispatch:

jobs:
  stale:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/stale@v3
      with:
        repo-token: ${{ secrets.GITHUB_TOKEN }}
        stale-pr-message: 'This Pull Request has been open for 2 days with no activity, if no additional activity is detected, it will be closed in 1 day.'
        days-before-stale: 2
        days-before-close: 1
        stale-pr-label: 'stale'
        exempt-pr-label: 'work in progress'

It marked the PR as stale after two days, and closed it after 2 days. The label was applied.
So, spaces in exempt-pr-label was not solved, and it is now only following days-before-stale count for closing.

@bacongobbler
Copy link

You need to upgrade to v3.0.14.

@brianeclow
Copy link
Author

So, using actions/stale@v3 does not keep me floating at the most current revision of version 3? This is the SOP for all other Actions.

@bacongobbler
Copy link

bacongobbler commented Nov 25, 2020

hmm... When it was first released, v3 was not updated to the new version. That seems to have changed and it's now pointing at the same commit as v3.0.14. Disregard.

@brianeclow
Copy link
Author

I am re-running the test with the above listed workflow.

@brianeclow
Copy link
Author

This is still running against PR's. Looks like the fix was for only issues. Can this be re-opened?

@brianeclow
Copy link
Author

However, it is looking like, the timing is off. It now is ignoring difference between:

        days-before-stale: 2
        days-before-close: 1

And it is closing the PR at 2 days instead of 1.

@brianeclow
Copy link
Author

This is still a problem.

pebenito added a commit to SELinuxProject/refpolicy that referenced this issue Apr 21, 2021
However, a bug prevents this from working on PRs, see actions/stale#98.

Signed-off-by: Chris PeBenito <[email protected]>
pebenito added a commit to pebenito/setools that referenced this issue Apr 21, 2021
However, a bug prevents this from working on PRs, see actions/stale#98.

Signed-off-by: Chris PeBenito <[email protected]>
pebenito added a commit to SELinuxProject/setools that referenced this issue Apr 21, 2021
However, a bug prevents this from working on PRs, see actions/stale#98.

Signed-off-by: Chris PeBenito <[email protected]>
0xC0ncord pushed a commit to 0xC0ncord/hardened-refpolicy that referenced this issue Jul 20, 2021
However, a bug prevents this from working on PRs, see actions/stale#98.

Signed-off-by: Chris PeBenito <[email protected]>
0xC0ncord pushed a commit to 0xC0ncord/hardened-refpolicy that referenced this issue Jul 21, 2021
However, a bug prevents this from working on PRs, see actions/stale#98.

Signed-off-by: Chris PeBenito <[email protected]>
perfinion pushed a commit to perfinion/hardened-refpolicy that referenced this issue Sep 5, 2021
However, a bug prevents this from working on PRs, see actions/stale#98.

Signed-off-by: Chris PeBenito <[email protected]>
Signed-off-by: Jason Zaman <[email protected]>
@agiudiceandrea
Copy link

Has this issue actually been fixed? If yes, what is the correct way to specify two (or more) labels containing spaces for the exempt-pr-label config option with stale@v5?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants