-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
workflow which ensures that talawa Api app starts in docker #2759
workflow which ensures that talawa Api app starts in docker #2759
Conversation
WalkthroughThis pull request introduces comprehensive Docker-related enhancements across multiple configuration files. The changes focus on improving the GitHub Actions workflow to include a new Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
366-372
: Use environment variable for health check URLThe health check URL is hardcoded. Consider using the
HEALTH_CHECK_URL
environment variable:-until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://talawa-api-dev:4000/health" 2>&1 || [ $timeout -eq 0 ]; do +until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "${HEALTH_CHECK_URL}/health" 2>&1 || [ $timeout -eq 0 ]; do🧰 Tools
🪛 actionlint (1.7.4)
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
[error] 366-366: trailing spaces
(trailing-spaces)
392-407
: Fix environment variables indentationThe
env:
section should be at the same level assteps:
:- env: - HEALTH_CHECK_URL: http://localhost:4000 - COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }} + env: + HEALTH_CHECK_URL: http://localhost:4000 + COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }}🧰 Tools
🪛 yamllint (1.35.1)
[error] 407-407: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 407-407: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
making changes as suggested |
…ansh-talawa-api into api_fail_docker merge# Please enter a commit message to explain why this merge is necessary,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
311-320
: Optimize cache configurationConsider adding package-lock.json to the cache key for better cache hits.
- name: Cache Node.js dependencies uses: actions/cache@v4 with: path: | ~/.npm node_modules - key: ${{ runner.os }}-docker-check-${{ hashFiles('**/package-lock.json') }} + key: ${{ runner.os }}-docker-check-node-${{ hashFiles('package-lock.json') }} restore-keys: | - ${{ runner.os }}-docker-check- + ${{ runner.os }}-docker-check-node-
333-337
: Improve error handling in container startupConsider capturing and displaying the exit code for better debugging.
-if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then +if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then + exit_code=$? echo "Failed to start containers" + echo "Exit code: $exit_code" docker-compose -f docker-compose.dev.yaml logs exit 1 fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 407-407: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
307-307
: Fix action step configuration syntax
The Node.js setup step has incorrect syntax.
-- uses: actions/setup-node@v4
+ - uses: actions/setup-node@v4
392-407
: Verify environment variable configuration
Please verify that these environment variables match the ones in docker-compose.dev.yaml.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 407-407: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
393-407
: LGTM: Environment variables are well configuredThe environment variables are properly set with appropriate use of secrets. Just remove trailing spaces at line 407.
- MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} - + MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}
299-407
: Consider enhancing the Docker health check implementationSome suggestions to improve the implementation:
- Consider using Docker's built-in HEALTHCHECK instruction in Dockerfile instead of shell-based health checks
- Consider implementing exponential backoff for health checks instead of fixed intervals
- Consider adding container resource limits to prevent resource exhaustion
These changes would make the health checks more robust and maintainable.
🧰 Tools
🪛 actionlint (1.7.4)
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
372-372: could not parse as YAML: yaml: line 372: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 323-323: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 408-408: trailing spaces
(trailing-spaces)
[error] 374-374: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
299-320
: LGTM: Job configuration follows best practices
The job configuration is well-structured with proper dependency management and caching strategy.
353-364
:
Fix Redis health check syntax
There's a syntax error in the if condition:
- if[ $timeout -eq 0 ]; then
+ if [ $timeout -eq 0 ]; then
Likely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
321-337
: Fix formatting and enhance error logging.
- Remove trailing spaces after
run :
and in the if condition- Add container logs to the error message when docker-compose file is not found
- run : | + run: | # Ensure no containers are running docker-compose -f docker-compose.dev.yaml down -v || true # Verify docker-compose file exists - if [ ! -f "docker-compose.dev.yaml" ]; then + if [ ! -f "docker-compose.dev.yaml" ]; then echo "Error: docker-compose.dev.yaml not found" + docker-compose -f docker-compose.dev.yaml logs exit 1 fi🧰 Tools
🪛 yamllint (1.35.1)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
366-382
: Enhance API health check implementation.Consider adding additional health check parameters to ensure comprehensive API readiness:
- Add request timeout to curl command
- Check for specific health check response content
- until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do + until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf --max-time 5 "http://localhost:4000/health" 2>&1 | grep -q '"status":"healthy"' || [ $timeout -eq 0 ]; do🧰 Tools
🪛 yamllint (1.35.1)
[error] 366-366: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
384-384: could not parse as YAML: yaml: line 384: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 409-409: trailing spaces
(trailing-spaces)
[error] 388-388: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
299-320
: LGTM! Job configuration follows best practices.
The job configuration is well-structured with proper dependency setup, Node.js configuration, and dependency caching.
299-409
: Overall implementation achieves the PR objective.
The Docker check implementation successfully verifies that the Talawa API starts correctly in a Docker environment. It includes proper health checks, timeouts, and cleanup procedures. The remaining issues are mostly syntax and formatting related, which have been addressed in previous comments.
🧰 Tools
🪛 actionlint (1.7.4)
384-384: could not parse as YAML: yaml: line 384: could not find expected ':'
(syntax-check)
🪛 yamllint (1.35.1)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 327-327: trailing spaces
(trailing-spaces)
[error] 366-366: trailing spaces
(trailing-spaces)
[error] 409-409: trailing spaces
(trailing-spaces)
[error] 388-388: syntax error: could not find expected ':'
(syntax)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)
367-380
: Fix indentation in API health check.The indentation is inconsistent in this section, which affects readability.
- # Wait for TALAWA API to be healthy - until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do - echo "Waiting for API to start... ($timeout seconds remaining)" - sleep 1 - ((timeout--)) - done + # Wait for TALAWA API to be healthy + until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do + echo "Waiting for API to start... ($timeout seconds remaining)" + sleep 1 + ((timeout--)) + done🧰 Tools
🪛 yamllint (1.35.1)
[error] 367-367: trailing spaces
(trailing-spaces)
394-409
: Fix trailing spaces in environment variables section.Remove trailing spaces to maintain consistent formatting.
- MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} - + MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}} +🧰 Tools
🪛 yamllint (1.35.1)
[warning] 395-395: wrong indentation: expected 6 but found 8
(indentation)
[error] 409-409: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
322-322: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 367-367: trailing spaces
(trailing-spaces)
[warning] 395-395: wrong indentation: expected 6 but found 8
(indentation)
[error] 409-409: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/pull-request.yml (5)
299-320
: LGTM! Job configuration follows best practices.
The job configuration is well-structured with proper dependency management, Node.js setup, and dependency caching.
321-338
: LGTM! Docker startup implementation is robust.
The implementation includes proper error handling, timeouts, and cleanup of existing containers.
🧰 Tools
🪛 actionlint (1.7.4)
322-322: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [
(shellcheck)
343-343
: Fix MongoDB health check syntax.
There's a syntax error in the until condition that needs to be fixed.
-until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok">/dev/null 2>&1 || [ $timeout -eq 0]; do
+until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok" >/dev/null 2>&1 || [ $timeout -eq 0 ]; do
360-365
: Fix Redis health check syntax and indentation.
There are syntax errors in the Redis health check implementation.
-if[ $timeout -eq 0 ]; then
+if [ $timeout -eq 0 ]; then
384-393
: LGTM! Cleanup implementation follows best practices.
The cleanup function correctly preserves the exit code and uses trap for proper cleanup on script exit.
There isn’t an issue assigned to you for this PR. Please follow the guidelines in our PR_GUIDELINES.md file. We have the procedures in place so that everyone has a fair chance of contributing. I will be closing this pull request. Please follow the procedures and resubmit when ready. |
@Cioppolo14 I accidently tagged the wrong issue, but I am assigned to this issue: #2638..please re-open this |
Please make sure all tests pass and coderabbit.ai approves the PR |
yes sir, will get this ready 5-6 days |
Hi @prayanshchh . Have you found any solution for |
is that so, then ig this should be a seperate issue then, can u give ur PR link |
Sure. I have opened an issue regarding this since a few others also reported the same in slack. |
If you need any assistance feel free to reach out to me @prayanshchh @bandhan-majumder . |
Please fix:
Make sure CodeRabbit.ai approves your changes |
should I remove eslint-disable statements from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please fix the lint check errors in this PR
- The aim of the year is to fix historically based practice
- We cannot merge with errors
Please make code rabbit approve your PR |
code rabbit already approves the PR, will make lint fixes and then get back to this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
setup.ts (1)
1017-1024
: Add validation for PWD input.Consider adding validation to ensure the provided PWD path exists and is accessible.
const { pwdVariable } = await inquirer.prompt({ type: "input", name: "pwdVariable", message: "Please enter the value for PWD (working directory for Docker setup):", default: ".", + validate: (input: string): boolean | string => { + try { + const fullPath = path.resolve(input); + if (!fs.existsSync(fullPath)) { + return "Directory does not exist. Please provide a valid path."; + } + fs.accessSync(fullPath, fs.constants.R_OK | fs.constants.W_OK); + return true; + } catch { + return "Directory is not accessible. Please check permissions."; + } + }, });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
setup.ts
(2 hunks)tsconfig.json
(1 hunks)
🔇 Additional comments (4)
tsconfig.json (1)
11-16
: LGTM! Path aliases improve code organization.
The addition of path aliases and noUncheckedSideEffectImports
enhances code maintainability and type safety.
setup.ts (3)
9-9
: LGTM! Imports updated to use new path aliases.
The imports have been properly updated to use the newly configured path aliases from tsconfig.json.
Also applies to: 14-33
1017-1024
: LGTM! PWD configuration added for Docker setup.
The PWD configuration is essential for Docker working directory setup. The default value "." is sensible for most use cases.
1017-1046
: Verify Docker container startup sequence.
The PWD configuration is part of Docker setup. Let's verify the container startup sequence works as expected.
✅ Verification successful
Let me gather more information about the Docker configuration and startup sequence.
Let me check one more aspect of the Docker configuration to ensure proper service dependencies.
Docker container startup sequence is properly configured
The verification of the Docker setup and PWD configuration shows that:
- The PWD variable is correctly configured in both
.env.sample
and used in the Docker compose file for the Caddy service - The container startup sequence is well-defined with proper health checks and dependencies:
talawa-api-dev
depends onmongo
,redis-stack-server
, andminio
services- MongoDB has a health check configured
- All services are connected through the
talawa-network
bridge network - Volume mounts and environment variables are properly configured
The PWD configuration in the setup script aligns with its usage in the Docker compose file for the Caddy service volume mounts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Docker container startup sequence and configuration
# Check if docker-compose.dev.yaml exists and contains required services
if [ ! -f "docker-compose.dev.yaml" ]; then
echo "Error: docker-compose.dev.yaml not found"
exit 1
fi
# Check if docker-compose.dev.yaml contains required services and PWD usage
rg -A 5 "working_dir|PWD" docker-compose.dev.yaml
# Check if .env.sample contains PWD variable
rg "^PWD=" .env.sample
Length of output: 215
Script:
#!/bin/bash
# Check Docker service definitions and dependencies
rg -A 10 "services:" docker-compose.dev.yaml
# Check health checks and container dependencies
rg "healthcheck|depends_on" docker-compose.dev.yaml
# Check if there are any Docker-related scripts or configurations
fd -e yml -e yaml -e dockerfile -e Dockerfile
Length of output: 478
Script:
#!/bin/bash
# Check complete service dependencies and environment configurations
cat docker-compose.dev.yaml
# Check if there are any Docker-related environment variables in the sample
rg "DOCKER|COMPOSE" .env.sample
# Look for any Docker-related scripts
fd setup docker
Length of output: 2201
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/pull-request.yml (1)
341-345
: Consider pinning Docker Compose version.For better reproducibility and to avoid potential breaking changes, consider pinning the Docker Compose version:
- name: Install Docker Compose run: | sudo apt-get update - sudo apt-get install -y docker-compose + sudo apt-get install -y docker-compose=1.29.2*
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml
(2 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 393-393: trailing spaces
(trailing-spaces)
[error] 398-398: trailing spaces
(trailing-spaces)
[error] 474-474: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/pull-request.yml (5)
317-321
: LGTM!
The job configuration follows GitHub Actions best practices with proper naming, dependencies, and runner specification.
415-424
: LGTM!
The cleanup function is well-implemented with proper exit code preservation and error handling.
425-439
: LGTM!
The environment variables are well-configured with secure token generation using GitHub-specific values for uniqueness.
372-374
:
Fix MongoDB service name inconsistency.
The service name used in the logs command doesn't match the one used in the health check:
echo "Fetching MongoDB logs..."
-docker-compose -f docker-compose.dev.yaml logs mongodb
+docker-compose -f docker-compose.dev.yaml logs mongo
Likely invalid or redundant comment.
398-404
: 🛠️ Refactor suggestion
Improve API health check reliability.
The current OPTIONS request might not accurately verify the GraphQL endpoint's functionality. Consider using a simple query instead:
-until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -v -X OPTIONS "http://talawa-api-dev:4000/graphql" 2>&1 || [ $timeout -eq 0 ]; do
+until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf -X POST "http://talawa-api-dev:4000/graphql" \
+ -H "Content-Type: application/json" \
+ -d '{"query":"{ __typename }"}' 2>&1 || [ $timeout -eq 0 ]; do
Likely invalid or redundant comment.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 398-398: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
vite.config.mts (1)
48-48
: Remove unnecessary whitespace or trailing line changes for cleaner diffs.
Although not critical, reducing extraneous changes helps keep diffs minimal.- +tests/utilities/loadSampleData.spec.ts (1)
18-18
: Use environment-based DB naming for improved configurability.Instead of specifying
"talawa-api"
as a hardcoded string, consider referencing a dedicated environment variable or configuration file for the database name to allow more flexible test setups and avoid accidental conflicts when using different environments (development, staging, production, etc.). For example:- await connect("talawa-api"); + await connect(process.env.TALAWA_DB_NAME || "talawa-api");tests/setup/checkExistingMongoDB.spec.ts (1)
12-13
: Consider using structured logging or removing unnecessary logs.Using
console.log
can clutter test outputs and potentially leak sensitive information. If logging is truly necessary for debugging, you might consider using a more robust logging framework or wrapping logs in conditionals.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/utilities/loadSampleData.ts
(3 hunks)tests/resolvers/Query/getVolunteerRanks.spec.ts
(2 hunks)tests/setup/checkExistingMongoDB.spec.ts
(2 hunks)tests/setup/mongoDB.spec.ts
(1 hunks)tests/utilities/loadSampleData.spec.ts
(1 hunks)vite.config.mts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utilities/loadSampleData.ts
🔇 Additional comments (6)
vite.config.mts (2)
2-2
: Use node:path
with caution if older Node.js versions are supported.
If the project's Node.js version is <16.0, node:path
might not work. Verify that your .nvmrc
or documentation clarifies the minimum Node.js version.
5-11
: Great addition of path aliases for quick imports.
These aliases streamline imports and improve code readability.
tests/setup/checkExistingMongoDB.spec.ts (1)
65-66
: Verify dependencies and environment configurations.
The new connection string with replica set and direct connection parameters is a good step for Docker-based setups. However, verify that all references to MONGO_DB_URL
elsewhere (e.g., in your Docker or CI config) consistently match these parameters to avoid connection issues.
tests/setup/mongoDB.spec.ts (1)
120-121
: Change appears consistent with your Docker configuration.
This updated connection string aligns with the replica set configuration introduced elsewhere. Good job keeping the test environment consistent with the actual runtime environment.
tests/resolvers/Query/getVolunteerRanks.spec.ts (2)
80-80
: Confirm the updated expectation for monthly timeframe.
The expected hours changed to 2, which differs from previously reported hours (e.g., 6 or 8). If this new value aligns with recent logic changes in createVolunteerAndActions
or getVolunteerRanks
, then it’s correct. Otherwise, please verify that the test data and function logic properly reflect real-world use of the monthly timeframe.
97-97
: Double-check alignment of yearly hours log.
Similarly, we've updated the expected value to 2 for the yearly timeframe. Ensure that the test setup and underlying business logic (in createVolunteerAndActions
or the resolver) truly reduce the hours to 2 for a yearly duration, as that might be unintended if you expected a higher accumulation over a full year.
@palisadoes PTAL, the patch looks good |
b64f105
into
PalisadoesFoundation:develop
@prayanshchh you have changed the superadmin password and why is that? |
sorry it was for testing purposes, will undo this immediately, shall I create new PR, or continue here @gautam-divyanshu |
@prayanshchh create a new pr and issue. |
yes |
What kind of change does this PR introduce?
Changing workflow to ensure that talawa api starts in docker
Issue Number:
#2638
#2598
Fixes #
Did you add tests for your changes?
No
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?
Summary by CodeRabbit
Release Notes
New Features
Infrastructure
Configuration
Development