remove constraint for True Tokenless #181

nora-shap · 2024-10-12T01:23:50Z

Description

True Tokenless is coming. For it to work, it needs git_service. Removing this conditional allows git_service to be included on more uploads so they will be eligible for TT.

codecov

The overall changes made in the diff seem to create a looser check for an 'uploadToken'. It is removing an additional condition of 'branch' breakdown. However, without context, it's not sure why this change is needed. In particular, the presence or absence of 'uploadToken' does not seem to be correlated with the 'gitService'. If there's a correlation, the 'gitService' should not be set if 'uploadToken' is set, or vice versa.

codecov · 2024-10-12T01:24:04Z

packages/bundler-plugin-core/src/utils/getPreSignedURL.ts

   * See: https://github.com/codecov/codecov-api/pull/741
   */
-  if (!uploadToken && serviceParams.branch?.includes(":")) {
+  if (!uploadToken) {


This check now only verifies the existence of an 'uploadToken'. It will enter the if block even if there is no ':' in the 'branch'. Consider rechecking the requirements for 'uploadToken' and 'branch'.

codecov · 2024-10-12T01:24:04Z

packages/bundler-plugin-core/src/utils/getPreSignedURL.ts

   */
-  if (!uploadToken && serviceParams.branch?.includes(":")) {
+  if (!uploadToken) {
    if (gitService) {


Here you are setting 'gitService' if 'uploadToken' is not available. The control flow suggests that these two fields are somehow interchangeable or correlated. However, there might not be any correlation between 'uploadToken' and 'gitService', consider segregating the conditions or explain the logical correlation between them.

codecov · 2024-10-12T01:26:26Z

❌ 11 Tests Failed:

Tests completed	Failed	Passed	Skipped
308	11	297	0

View the top 3 failed tests by shortest run time

src/utils/__tests__/getPreSignedURL.test.ts getPreSignedURL &gt; unsuccessful request &gt; returned data is undefined &gt; throws an error

Stack Traces | 0.001s run time

AssertionError: expected Error: No upload token provided to be an instance of FailedFetchError
 ❯ .../utils/__tests__/getPreSignedURL.test.ts:396:23

src/utils/__tests__/getPreSignedURL.test.ts getPreSignedURL &gt; unsuccessful request &gt; using oidc and not github actions &gt; throws an error

Stack Traces | 0.001s run time

AssertionError: expected Error: No upload token provided to be an instance of BadOIDCServiceError
 ❯ .../utils/__tests__/getPreSignedURL.test.ts:488:23

src/utils/__tests__/getPreSignedURL.test.ts getPreSignedURL &gt; successful request &gt; when the initial response is successful &gt; &quot;uploadToken&quot; is not provided &gt; user provides &quot;gitService&quot; &gt; returns the pre-signed URL with the passed&quot;git_service&quot; in the request body

Stack Traces | 0.002s run time

Error: No upload token provided
 ❯ Module.getPreSignedURL src/utils/getPreSignedURL.ts:91:11
 ❯ .../utils/__tests__/getPreSignedURL.test.ts:182:31

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

codecov-staging · 2024-10-12T01:26:29Z

❌ 11 Tests Failed:

Tests completed	Failed	Passed	Skipped
308	11	297	0

View the top 3 failed tests by shortest run time

src/utils/__tests__/getPreSignedURL.test.ts getPreSignedURL &gt; unsuccessful request &gt; returned data is undefined &gt; throws an error

Stack Traces | 0.001s run time

AssertionError: expected Error: No upload token provided to be an instance of FailedFetchError
 ❯ .../utils/__tests__/getPreSignedURL.test.ts:396:23

src/utils/__tests__/getPreSignedURL.test.ts getPreSignedURL &gt; unsuccessful request &gt; using oidc and not github actions &gt; throws an error

Stack Traces | 0.001s run time

AssertionError: expected Error: No upload token provided to be an instance of BadOIDCServiceError
 ❯ .../utils/__tests__/getPreSignedURL.test.ts:488:23

src/utils/__tests__/getPreSignedURL.test.ts getPreSignedURL &gt; unsuccessful request &gt; fetch throws an error &gt; throws an error

Stack Traces | 0.002s run time

AssertionError: expected Error: No upload token provided to be an instance of FailedFetchError
 ❯ .../utils/__tests__/getPreSignedURL.test.ts:458:23

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

codecov

CodecovAI submitted a new review for 0a89a12

codecov · 2024-10-12T01:47:59Z

packages/bundler-plugin-core/src/utils/getPreSignedURL.ts

-   * proper tokenless upload.
-   * See: https://github.com/codecov/codecov-api/pull/741
-   */
-  if (!uploadToken && serviceParams.branch?.includes(":")) {


Removing this condition means the format of the branch is no longer checked. Has the branch check been moved elsewhere, or is it no longer needed? If not necessary anymore, kindly disregard the comment.

codecov · 2024-10-12T01:47:59Z

packages/bundler-plugin-core/src/utils/getPreSignedURL.ts

-      const foundGitService = findGitService();
-      if (!foundGitService || foundGitService === "") {
+
+  if (gitService) {


This gitService check is used to set the requestBody.git_service. However, what happens when the gitService isn't provided or found? Ensure fallback or error handling measures are in place.

codecov · 2024-10-12T01:47:59Z

packages/bundler-plugin-core/src/utils/getPreSignedURL.ts

+  if (gitService) {
+    requestBody.git_service = gitService;
+  } else {
+    const foundGitService = findGitService();


The findGitService function is called here if gitService is not provided. This is good since it provides a fallback for gitService.

codecov · 2024-10-12T01:47:59Z

packages/bundler-plugin-core/src/utils/getPreSignedURL.ts

+    requestBody.git_service = gitService;
+  } else {
+    const foundGitService = findGitService();
+      if (!foundGitService || foundGitService === "" && !uploadToken) {


This condition checks again if gitService exists. However, aren't we already in a code block where we know gitService doesn't exist? This check seems redundant and may lead to unnecessary computational costs.

codecov · 2024-10-12T01:47:59Z

packages/bundler-plugin-core/src/utils/getPreSignedURL.ts

-    }
-  } else if (oidc?.useGitHubOIDC && Core) {
+  }
+  if (oidc?.useGitHubOIDC && Core) {


The condition 'oidc?.useGitHubOIDC && Core' is left unchanged but with updated indentation level. This alteration shows good git practice. However, its purpose isn't clear without context, make sure 'Core' is defined somewhere above the code block. If 'Core' is not defined elsewhere, there will be a reference error at runtime.

nicholas-codecov · 2024-11-14T11:37:00Z

@nora-codecov is this PR still active or are we good to close in favour of #189 ?

remove constraint for True Tokenless

2725c9f

codecov bot reviewed Oct 12, 2024

View reviewed changes

change logic around

0a89a12

codecov bot reviewed Oct 12, 2024

View reviewed changes

nora-shap closed this Nov 14, 2024

remove constraint for True Tokenless #181

remove constraint for True Tokenless #181

Uh oh!

Conversation

nora-shap commented Oct 12, 2024

Description

Uh oh!

codecov bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

codecov bot Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 11 Tests Failed:

Uh oh!

codecov-staging bot commented Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 11 Tests Failed:

Uh oh!

codecov bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

codecov bot Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

codecov bot Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

codecov bot Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

codecov bot Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

nicholas-codecov commented Nov 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Oct 12, 2024 •

edited

Loading

codecov-staging bot commented Oct 12, 2024 •

edited

Loading