Repository: codecov/uploader
Pull request #857 (2022-08-22 22:05:37)

Don't call `checkSlug` if a token is passed
It's not necessary to validate `slug` if a token is passed in; the server can determine it from the upload token.

codecov[bot]: # [Codecov](https://app.codecov.io/gh/codecov/uploader/compare/857?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codecov) Report
> Merging [#857](https://app.codecov.io/gh/codecov/uploader/compare/857?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codecov) (6d392cb) into [main](https://codecov.io/gh/codecov/uploader/commit/0e6c734109c78ca4ed746a05a1ff9be24b33c361?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codecov) (0e6c734) will **decrease** coverage by `0.14%`.
> The diff coverage is `50.00%`.

> :exclamation: Current head 6d392cb differs from pull request most recent head 36b2343. Consider uploading reports for the commit 36b2343 to get more accurate results

```diff
@@            Coverage Diff             @@
##             main     #857      +/-   ##
==========================================
- Coverage   91.87%   91.72%   -0.15%     
==========================================
  Files          34       34              
  Lines        1181     1184       +3     
  Branches      244      246       +2     
==========================================
+ Hits         1085     1086       +1     
- Misses         64       65       +1     
- Partials       32       33       +1     
```

| Flag | Coverage Δ | |
|---|---|---|
| alpine | `91.72% <50.00%> (-0.15%)` | :arrow_down: |
| alpine-proxy | `91.72% <50.00%> (-0.15%)` | :arrow_down: |
| alpine-without-git | `91.72% <50.00%> (-0.15%)` | :arrow_down: |
| linux | `91.72% <50.00%> (-0.15%)` | :arrow_down: |
| linux-without-git | `91.72% <50.00%> (-0.15%)` | :arrow_down: |
| macos | `91.72% <50.00%> (-0.15%)` | :arrow_down: |
| macos-without-git | `91.72% <50.00%> (-0.15%)` | :arrow_down: |
| windows | `91.72% <50.00%> (-0.15%)` | :arrow_down: |
| windows-without-git | `91.72% <50.00%> (-0.15%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codecov#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://app.codecov.io/gh/codecov/uploader/compare/857?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codecov) | Coverage Δ | |
|---|---|---|
| [src/index.ts](https://app.codecov.io/gh/codecov/uploader/compare/857/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codecov#diff-c3JjL2luZGV4LnRz) | `73.58% <33.33%> (-0.78%)` | :arrow_down: |
| [src/helpers/checkSlug.ts](https://app.codecov.io/gh/codecov/uploader/compare/857/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=codecov#diff-c3JjL2hlbHBlcnMvY2hlY2tTbHVnLnRz) | `100.00% <100.00%> (ø)` | |

thomasrockhu-codecov: Could probably use some tests here
lgtm-com[bot]: This pull request **introduces 4 alerts** when merging dc9944b2cd7f06f0b3e73a7ac6d4d6182fdefc60 into 1734fdbd873817c40081c2e60f5ade7f17258fab - [view on LGTM.com](https://lgtm.com/projects/g/codecov/uploader/rev/pr-38deea1309b8ae69c859745b6f04834928d73cc0)

**new alerts:**

* 3 for Useless assignment to local variable
* 1 for Unreachable statement
lgtm-com[bot]: This pull request **introduces 3 alerts** when merging b5f5966fa9fbd9de0e5909194177cc859cb9b7e4 into 1734fdbd873817c40081c2e60f5ade7f17258fab - [view on LGTM.com](https://lgtm.com/projects/g/codecov/uploader/rev/pr-1b482688ef1253957d4b81e20c0ff38b59350479)

**new alerts:**

* 1 for Useless assignment to local variable
* 1 for Unreachable statement
* 1 for Unneeded defensive code
lgtm-com[bot]: This pull request **introduces 3 alerts** when merging 6924992254f782efc9d5042abc30122faaf21586 into 1734fdbd873817c40081c2e60f5ade7f17258fab - [view on LGTM.com](https://lgtm.com/projects/g/codecov/uploader/rev/pr-7a68a8e9c08e895caf2afb12b56c2b6a660da092)

**new alerts:**

* 2 for Unreachable statement
* 1 for Unneeded defensive code
lgtm-com[bot]: This pull request **introduces 6 alerts** when merging 1a85e60531b6793c0f2e58da35ff558b18b812b6 into 1734fdbd873817c40081c2e60f5ade7f17258fab - [view on LGTM.com](https://lgtm.com/projects/g/codecov/uploader/rev/pr-d313fad54a89bad44159f161639fda97764abf9b)

**new alerts:**

* 3 for Useless assignment to local variable
* 2 for Unreachable statement
* 1 for Unneeded defensive code
lgtm-com[bot]: This pull request **introduces 6 alerts** when merging ecf8cbe83e318cfafb086d9d99cb1c6cd9572c3f into 1734fdbd873817c40081c2e60f5ade7f17258fab - [view on LGTM.com](https://lgtm.com/projects/g/codecov/uploader/rev/pr-d856b4ff2097d53d480b7eb8386b8b765facc76b)

**new alerts:**

* 3 for Useless assignment to local variable
* 2 for Unreachable statement
* 1 for Unneeded defensive code
lgtm-com[bot]: This pull request **introduces 6 alerts** when merging c499dd04d0bb154ec005de21c4a9d1fbc30571f8 into 1734fdbd873817c40081c2e60f5ade7f17258fab - [view on LGTM.com](https://lgtm.com/projects/g/codecov/uploader/rev/pr-d594ee2cfdc4e66cdba80150404dcd2d1eef8166)

**new alerts:**

* 3 for Useless assignment to local variable
* 2 for Unreachable statement
* 1 for Unneeded defensive code
lgtm-com[bot]: This pull request **introduces 6 alerts** when merging 8bc1f7d72378fca94f60ade3364df9fd99ad35af into 1734fdbd873817c40081c2e60f5ade7f17258fab - [view on LGTM.com](https://lgtm.com/projects/g/codecov/uploader/rev/pr-04cf60f9ad0744fba583bc66f65a67fa19c56eab)

**new alerts:**

* 3 for Useless assignment to local variable
* 2 for Unreachable statement
* 1 for Unneeded defensive code

Patch:
diff --git a/.nvmrc b/.nvmrc
index c85fa1b..b26a239 100644
--- a/.nvmrc
+++ b/.nvmrc
@@ -1,1 +1,1 @@
-16.17.1
+v17.9.1
diff --git a/package.json b/package.json
index e13d83d..2a76976 100644
--- a/package.json
+++ b/package.json
@@ -75,5 +75,8 @@
     "**/*.{json,yml,yaml,html}": [
       "prettier --write"
     ]
+  },
+  "volta": {
+    "node": "17.9.1"
   }
 }
diff --git a/src/helpers/checkSlug.ts b/src/helpers/checkSlug.ts
index dc7ff2a..e42a6fa 100644
--- a/src/helpers/checkSlug.ts
+++ b/src/helpers/checkSlug.ts
@@ -1,5 +1,5 @@
-export function checkSlug(slug: string | undefined): boolean {
-  if (slug !== '' && !slug?.match(/\//)) {
+export function checkSlug(slug: string): boolean {
+  if (slug !== '' && !slug.match(/\//)) {
     return false
   }
   return true
diff --git a/src/index.ts b/src/index.ts
index 175c22c..347694a 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -353,9 +353,19 @@ export async function main(
     UploadLogger.verbose(`${parameter}`)
   }
 
-  const validSlug =  checkSlug(buildParams.slug)
-  if (!validSlug) {
-    throw new Error(`Slug must follow the format of "<owner>/<repo>" or be blank. We detected "${buildParams.slug}"`)
+  if (!hasToken) {
+    if (!buildParams.slug) {
+      throw new Error(
+        'Slug must be set if a token is not passed. Consider passing a slug via `-r`',
+      )
+    } else {
+      const validSlug = checkSlug(buildParams.slug)
+      if (!validSlug) {
+        throw new Error(
+          `Slug must follow the format of "<owner>/<repo>". We detected "${buildParams.slug}"`,
+        )
+      }
+    }
   }
 
   const query = webHelpers.generateQuery(buildParams)


Test patch:
diff --git a/test/helpers/checkSlug.test.ts b/test/helpers/checkSlug.test.ts
index 875ae15..7dffab8 100644
--- a/test/helpers/checkSlug.test.ts
+++ b/test/helpers/checkSlug.test.ts
@@ -24,16 +24,4 @@ describe('checkSlug()', () => {
         // assert
         expect(result).toEqual(expectedResult)
     })
-
-    it('should return false for a slug that is "undefined"', () => {
-        // arrange
-        const inputSlug = undefined
-        const expectedResult = false
-
-        // act
-        const result = checkSlug(inputSlug)
-
-        // assert
-        expect(result).toEqual(expectedResult)
-    })
-})
\ No newline at end of file
+})
diff --git a/test/index.test.ts b/test/index.test.ts
index ea701a0..4b4630c 100644
--- a/test/index.test.ts
+++ b/test/index.test.ts
@@ -12,8 +12,9 @@ import * as app from '../src'
 
 import { version } from '../package.json'
 import { UploadLogger } from '../src/helpers/logger'
-import { detectProvider } from '../src/helpers/provider'
+import * as providerHelpers from '../src/helpers/provider'
 import * as webHelpers from '../src/helpers/web'
+import { IServiceParams } from '../src/types'
 
 // Backup the env
 const realEnv = { ...process.env }
@@ -78,7 +79,7 @@ describe('Uploader Core', () => {
       url: 'https://codecov.io',
     }
     const inputs = { args, environment: process.env }
-    const serviceParams = detectProvider(inputs, args.token != '')
+    const serviceParams = providerHelpers.detectProvider(inputs, args.token != '')
     const buildParams = webHelpers.populateBuildParams(inputs, serviceParams)
     const query = webHelpers.generateQuery(buildParams)
 
@@ -132,7 +133,7 @@ describe('Uploader Core', () => {
       jest.clearAllMocks()
     })
 
-    it('Can upload without token', async () => {
+    it('Can upload without token if slug is passed', async () => {
       jest.spyOn(process, 'exit')
       const log = jest.spyOn(console, 'log').mockImplementation(() => {
         // intentionally empty
@@ -143,13 +144,42 @@ describe('Uploader Core', () => {
         dryRun: 'true',
         env: 'SOMETHING,ANOTHER',
         flags: '',
-        slug: '',
+        slug: 'codecov/uploader',
         upstream: ''
       })
       expect(log).toHaveBeenCalledWith(
         expect.stringMatching('-> No token specified or token is empty'),
       )
     })
+
+    it('Cannot upload without token if slug is not passed', async () => {
+      const f = () => app.main({
+        name: 'customname',
+        url: 'https://codecov.io',
+        dryRun: 'true',
+        env: 'SOMETHING,ANOTHER',
+        flags: '',
+        slug: '',
+        upstream: ''
+      })
+
+      const detectProvider = td.replace(providerHelpers, 'detectProvider')
+      const buildParams: Partial<IServiceParams> = {
+        branch: '',
+        build: '',
+        buildURL: '',
+        commit: 'testSHA',
+        job: '',
+        pr: '',
+        service: 'no service',
+        slug: '',
+      }
+      td.when(detectProvider(td.matchers.anything(), false)).thenReturn(buildParams)
+
+      await expect(f()).rejects.toThrow(
+        'Slug must be set if a token is not passed',
+      )
+    })
   })
 
   describe('Flags', () => {
@@ -167,7 +197,7 @@ describe('Uploader Core', () => {
         upstream: ''
       }
       const inputs = { args, environment: process.env }
-      const serviceParams = detectProvider(inputs, args.token != '')
+      const serviceParams = providerHelpers.detectProvider(inputs, args.token != '')
       const buildParams = webHelpers.populateBuildParams(inputs, serviceParams)
       const query = webHelpers.generateQuery(buildParams)
 
@@ -204,7 +234,7 @@ describe('Uploader Core', () => {
       upstream: ''
     }
     const inputs = { args, environment: process.env }
-    const serviceParams = detectProvider(inputs, args.token != '')
+    const serviceParams = providerHelpers.detectProvider(inputs, args.token != '')
     const buildParams = webHelpers.populateBuildParams(inputs, serviceParams)
     const query = webHelpers.generateQuery(buildParams)