Skip to content

Conversation

@ayush4345
Copy link
Collaborator

No description provided.

Copy link
Owner

@mittal-parth mittal-parth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't reviewed the code yet
Will do once we have the build status figured out

PORT: ${{ secrets.PORT }}
HUDDLE_PROJECT_ID: ${{ secrets.HUDDLE_PROJECT_ID }}
HUDDLE_API_KEY: ${{ secrets.HUDDLE_API_KEY }}
PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this to LIT_WALLET_PRIVATE_LEY

Suggested change
PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }}
LIT_WALLET_PRIVATE_LEY: ${{ secrets.LIT_WALLET_PRIVATE_LEY }}

@netlify
Copy link

netlify bot commented Dec 4, 2025

👷 Deploy Preview for khoj-alpha processing.

Name Link
🔨 Latest commit 3b1d80a
🔍 Latest deploy log https://app.netlify.com/projects/khoj-alpha/deploys/6939196393e2020008cdae3a

@ayush4345
Copy link
Collaborator Author

@copilot make changes so that test related to sign protocol get ignored while running test

Copy link
Contributor

Copilot AI commented Dec 4, 2025

@ayush4345 I've opened a new pull request, #187, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 16 commits December 4, 2025 09:37
backend: configure Jest to skip Sign Protocol tests
- Remove built-in cache from setup-node to prevent yarn cache path issues
- Add explicit npm cache directory creation step
- Add actions/cache@v3 with proper npm paths and cache keys
- Update install step to conditionally use npm ci or npm install
- Set node-version explicitly to '24' instead of matrix variable
- Apply changes to both test and integration-test jobs

Fixes CI job 57226870056 failure: "Some specified paths were not resolved, unable to cache dependencies"

Co-authored-by: ayush4345 <[email protected]>
…s-to-use-npm

Fix backend-tests workflow npm caching to prevent "paths not resolved" error
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have all these dependencies and scripts added but not no tests?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont see the need to alter this file. I wouldn't want to alter this for tests to work - quite sensitive

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't have a separate file for this

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are actually not integration tests

They are mostly just unit tests checking each API. Integration tests are one that test everything. In our case everything happening from the UI interaction via the API, and contract should all be tested.

This is also good to have but these are not integration tests.

Plus, we can't have response to be either 200 or 400/500 as it in most of these tests. We should mock the services and then expect the exact response - both 200 and 400/500 in separate tests. These will still be unit tests however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants