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.

Sign up for GitHub

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

Jump to bottom

unreliable test: "crossFileContextUtil full support should be non empty" #6252

Closed
justinmk3 opened this issue Dec 16, 2024 · 2 comments
Closed

unreliable test: "crossFileContextUtil full support should be non empty" #6252

justinmk3 opened this issue Dec 16, 2024 · 2 comments
Labels
tests-ci-cd

Comments

@justinmk3
Copy link
Contributor

justinmk3 commented Dec 16, 2024 •
edited
Loading

Test Details

Log of Test Failure

  1) crossFileContextUtil
       full support
         should be non empty:

      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert_1.default.ok(actual && actual.supplementalContextItems.length !== 0)

      + expected - actual

      -false
      +true
      
      at Context.<anonymous> (test/unit/codewhisperer/util/crossFileContextUtil.test.ts:342:24)
      at runNextTicks (node:internal/process/task_queues:60:5)
      at processTimers (node:internal/timers:516:9)

  2) crossFileContextUtil
       full support
         should be non empty:

      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert_1.default.ok(actual && actual.supplementalContextItems.length !== 0)

      + expected - actual

      -false
      +true
      
      at Context.<anonymous> (test/unit/codewhisperer/util/crossFileContextUtil.test.ts:342:24)

Related
@justinmk3
Copy link
Contributor Author

Also sometimes the openTabsContext case:

 1) supplementalContextUtil
       fetchSupplementalContext
         openTabsContext
           opentabContext should include chunks if non empty:

      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert_1.default.ok(actual?.supplementalContextItems.length === 3)

      + expected - actual

      -false
      +true
      
      at Context.<anonymous> (test/unit/codewhisperer/util/supplemetalContextUtil.test.ts:45:24)
      at runNextTicks (node:internal/process/task_queues:60:5)
      at processTimers (node:internal/timers:516:9)

@justinmk3 justinmk3 mentioned this issue Dec 16, 2024
Merged
@Will-ShaoHua Will-ShaoHua mentioned this issue Dec 16, 2024
Merged
Will-ShaoHua added a commit that referenced this issue Dec 16, 2024
@Will-ShaoHua
fix(inline-suggestion): replace vscode.cancellation with waitUntil fo…
ad52466 
…r timeout (#6256)

## Problem

related issue: #6079,
#6252

caller
```
function main () {
       // init vscode cancellation token
       const cancellationToken
       setTimeout(100, () => {
              cancellationToken.cancel()
       })

      highlevelWrapperFetchSupplementalContext(editor, cancellationToken)
}

```

```
export function highlevelWrapperFetchSupplementalContext(editor, cancellationToken) {
       const supplementalContext = waitUntil(100, () => {
            // here always timeout and throw TimeoutException
            const opentabs =  await fetchOpenTabsContext(...)
            const projectContext = await fetchProjectContext()

           const result = []
           if (projectContext not empty) {
                    // push project context
           }

           if (opentabs not empty) {}
                   // push openttabs
           })  
          

         return result
}


async function fetchOpenTabsContext(editor, cancellationToken) {
      ....
      // VSC api call
}

async function fetchProjectContext() {
     ....
     // LSP call
}

```



After investigation, it looks like mix use of `vscode.CancellationToken`
and `waitUntil()` will likely cause cancellation token to be cancelled
prematurely (might be because another layer of waitUntil will run the
fetchOpenTabsContext asynchronously thus causing it to timeout
prematurely) therefore `fetchOpebtabsContext(..)` will return null in
this case and hence causing test cases failing.

Therefore, the issue here is actually not the test case itself and
they're failing due to race condition

## Solution
remove usage of cancellation token and only use waitUntil for timeout
purpose








## Functional testing

retrieved sup context as expected





### Case 1: repomap is available (there are local imports)
```
2024-12-16 13:10:15.616 [debug] CodeWhispererSupplementalContext:
    isUtg: false,
    isProcessTimeout: false,
    contentsLength: 14436,
    latency: 16.67179101705551
    strategy: codemap
    Chunk 0:
        Path: q-inline
        Length: 10209
        Score: 0
    Chunk 1:
        Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/service/serviceContainer.ts
        Length: 1486
        Score: 22.60257328587725
    Chunk 2:
        Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts
        Length: 1649
        Score: 19.106700952807103
    Chunk 3:
        Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts
        Length: 1092
        Score: 10.334690655691002
```

### Case 2: No repomap, should fallback to opentabs only

![image](https://github.com/user-attachments/assets/f59c11cf-0e34-40b8-8162-34b4d057673f)

```
2024-12-16 13:11:29.738 [debug] CodeWhispererSupplementalContext:
    isUtg: false,
    isProcessTimeout: false,
    contentsLength: 5046,
    latency: 16.311500012874603
    strategy: opentabs
    Chunk 0:
        Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts
        Length: 1564
        Score: 0
    Chunk 1:
        Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts
        Length: 1649
        Score: 0
    Chunk 2:
        Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts
        Length: 1833
        Score: 0
```





---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
@Will-ShaoHua
Copy link
Contributor

Will-ShaoHua commented Dec 16, 2024 •
edited
Loading

temporarily skipped test case other than java and only run 1 language test case (by running only 1 language, test went through well), still looking into test cases failure but have tested functionally and works fine on the languages skipped

@Will-ShaoHua Will-ShaoHua mentioned this issue Dec 17, 2024
Merged
justinmk3 pushed a commit that referenced this issue Dec 17, 2024
@Will-ShaoHua
test(inline-suggestion): crossFileContextUtil.test using real clock a…
7084800 
…nd test timeouts #6258

## Problem
ref #6252
Test is using real system clock, it's likely to timeout
due to heavy computation in an undeterministic way.

## Solution
Use fake clock.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
tests-ci-cd
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@justinmk3 @Will-ShaoHua