Skip to content

Conversation

@mattia1208
Copy link
Contributor

@mattia1208 mattia1208 commented Nov 7, 2025

Issue

https://github.com/parse-community/parse-server/issues/9907

Approach

This PR adds a failing test that demonstrates an issue where the environment variable
PARSE_SERVER_AUTH_PROVIDERS is not parsed as a JSON object, but kept as a raw string when Parse Server is launched via the CLI (bin/parse-server).

Summary by CodeRabbit

  • Tests
    • Expanded test coverage for authentication provider configuration parsing from environment variables.
    • Added tests for standalone parsing logic, CLI integration, and JSON string input handling.

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title test: add failing test for PARSE_SERVER_AUTH_PROVIDERS env var (string vs object) test: Add failing test for PARSE_SERVER_AUTH_PROVIDERS env var (string vs object) Nov 7, 2025
@parse-github-assistant
Copy link

🚀 Thanks for opening this pull request!

@coderabbitai
Copy link

coderabbitai bot commented Nov 7, 2025

📝 Walkthrough

Walkthrough

Three test suites are added to verify standalone parsing of Parse Server auth providers and environment variable handling. Tests validate JSON string parsing, object structure, error conditions, and integration with environment variables.

Changes

Cohort / File(s) Summary
Test Coverage
spec/AuthParserStandalone.spec.js, spec/CLI.spec.js, spec/parsers.spec.js
New test suite added to verify standalone auth provider parsing with JSON strings and objects; validates error handling, environment variable reading, and parser integration. Additional test cases added to existing test suites for environment variable parsing and objectParser with JSON string input.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • AuthParserStandalone.spec.js requires verification that all test scenarios correctly validate the parsing behavior and that conditional logic for the action parser is properly implemented
  • CLI.spec.js and parsers.spec.js additions are straightforward single test case extensions

Possibly related issues

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a failing test that demonstrates the PARSE_SERVER_AUTH_PROVIDERS environment variable parsing issue (string vs object).
Description check ✅ Passed The description provides the issue reference and approach, but omits the 'Tasks' section checklist required by the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@parseplatformorg
Copy link
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copy link

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5e7d6e and 126e1ec.

📒 Files selected for processing (3)
  • spec/AuthParserStandalone.spec.js (1 hunks)
  • spec/CLI.spec.js (1 hunks)
  • spec/parsers.spec.js (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:446-454
Timestamp: 2025-08-27T09:08:34.252Z
Learning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Applied to files:

  • spec/CLI.spec.js
  • spec/parsers.spec.js
  • spec/AuthParserStandalone.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Applied to files:

  • spec/CLI.spec.js
  • spec/AuthParserStandalone.spec.js
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.

Applied to files:

  • spec/CLI.spec.js
  • spec/AuthParserStandalone.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Applied to files:

  • spec/CLI.spec.js
  • spec/AuthParserStandalone.spec.js
🧬 Code graph analysis (2)
spec/CLI.spec.js (2)
resources/buildConfigDefinitions.js (1)
  • env (107-107)
spec/helper.js (1)
  • databaseURI (66-66)
spec/AuthParserStandalone.spec.js (1)
spec/parsers.spec.js (1)
  • require (1-10)
🪛 GitHub Check: Lint
spec/AuthParserStandalone.spec.js

[failure] 83-83:
Trailing spaces not allowed


[failure] 78-78:
Trailing spaces not allowed


[failure] 74-74:
Trailing spaces not allowed


[failure] 66-66:
Trailing spaces not allowed


[failure] 58-58:
Trailing spaces not allowed


[failure] 46-46:
Trailing spaces not allowed


[failure] 44-44:
Trailing spaces not allowed


[failure] 24-24:
Trailing spaces not allowed


[failure] 22-22:
Trailing spaces not allowed


[failure] 11-11:
Trailing spaces not allowed

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: Node 22
  • GitHub Check: Redis Cache
  • GitHub Check: Node 18
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: Node 20
  • GitHub Check: Docker Build
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: MongoDB 6, ReplicaSet
🔇 Additional comments (1)
spec/parsers.spec.js (1)

84-107: Expanded objectParser coverage looks solid. Exercising both the JSON-string input and the pre-built object ensures we lock in the expected behavior for auth configuration parsing. Nicely done.

Comment on lines 11 to 120

it('should parse auth providers JSON string to object with objectParser', () => {
const authProvidersString = JSON.stringify({
facebook: {
appIds: ['test-facebook-app-id-1', 'test-facebook-app-id-2']
},
google: {
clientId: 'test-google-client-id',
clientSecret: 'test-google-client-secret'
}
});

const result = parsers.objectParser(authProvidersString);

expect(typeof result).toBe('object');
expect(result).not.toBeNull();
expect(result.facebook).toBeDefined();
expect(result.facebook.appIds).toEqual(['test-facebook-app-id-1', 'test-facebook-app-id-2']);
expect(result.google).toBeDefined();
expect(result.google.clientId).toBe('test-google-client-id');
expect(result.google.clientSecret).toBe('test-google-client-secret');
});

it('should accept auth providers as object directly with objectParser', () => {
const authProvidersObject = {
facebook: {
appIds: ['direct-facebook-app-id']
},
twitter: {
consumerKey: 'test-consumer-key',
consumerSecret: 'test-consumer-secret'
}
};

const result = parsers.objectParser(authProvidersObject);

expect(typeof result).toBe('object');
expect(result.facebook).toBeDefined();
expect(result.facebook.appIds).toEqual(['direct-facebook-app-id']);
expect(result.twitter).toBeDefined();
expect(result.twitter.consumerKey).toBe('test-consumer-key');
});

it('should throw error for invalid JSON string', () => {
expect(() => {
parsers.objectParser('invalid-json-string-not-json');
}).toThrow();

expect(() => {
parsers.objectParser('just a plain string');
}).toThrow();
});

it('should verify auth definition in ParseServerOptions', () => {
const authDefinition = definitions.ParseServerOptions.auth;

expect(authDefinition).toBeDefined();
expect(authDefinition.env).toBe('PARSE_SERVER_AUTH_PROVIDERS');
expect(authDefinition.help).toContain('authentication providers');
});

it('CRITICAL TEST: auth definition should have objectParser action', () => {
const authDefinition = definitions.ParseServerOptions.auth;

// Questo è il test più importante: verifica che ci sia l'action parser
if (authDefinition.action) {
// Se c'è l'action, testiamo che funzioni correttamente
const testAuthString = JSON.stringify({
facebook: { appIds: ['test'] },
google: { clientId: 'test-client' }
});
const result = authDefinition.action(testAuthString);

expect(typeof result).toBe('object');
expect(result.facebook).toBeDefined();
expect(result.google).toBeDefined();

// Testa anche che accetti oggetti direttamente
const testAuthObject = { facebook: { appIds: ['test2'] } };
const result2 = authDefinition.action(testAuthObject);
expect(result2.facebook.appIds).toEqual(['test2']);

} else {
// Se non c'è l'action, questo test fallisce intenzionalmente
// per evidenziare il problema
fail('PROBLEMA TROVATO: auth definition manca di action parser! ' +
'Quando PARSE_SERVER_AUTH_PROVIDERS viene passata come variabile d\'ambiente, ' +
'verrà trattata come stringa invece che come oggetto. ' +
'Aggiungi "action: parsers.objectParser" alla definizione di auth in Definitions.js');
}
});

it('should demonstrate the problem: environment variable stays as string without parser', () => {
const authDefinition = definitions.ParseServerOptions.auth;

// Simula quello che succede quando viene passata una variabile d'ambiente
const envValue = '{"facebook":{"appIds":["test"]}}';

if (!authDefinition.action) {
// Senza action parser, la stringa rimane stringa
expect(typeof envValue).toBe('string');
console.log('⚠️ PROBLEMA: La variabile d\'ambiente PARSE_SERVER_AUTH_PROVIDERS ' +
'viene passata come stringa e non viene parsata in oggetto!');
} else {
// Con action parser, viene convertita in oggetto
const parsed = authDefinition.action(envValue);
expect(typeof parsed).toBe('object');
console.log('✓ OK: La variabile d\'ambiente viene correttamente parsata in oggetto');
}
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove trailing whitespace to satisfy lint. The linter flags trailing spaces on multiple lines (for example lines 11, 22, 24, 44, 46, 58, 66, 74, 78, 83). Please trim them so the lint check passes.

🧰 Tools
🪛 GitHub Check: Lint

[failure] 83-83:
Trailing spaces not allowed


[failure] 78-78:
Trailing spaces not allowed


[failure] 74-74:
Trailing spaces not allowed


[failure] 66-66:
Trailing spaces not allowed


[failure] 58-58:
Trailing spaces not allowed


[failure] 46-46:
Trailing spaces not allowed


[failure] 44-44:
Trailing spaces not allowed


[failure] 24-24:
Trailing spaces not allowed


[failure] 22-22:
Trailing spaces not allowed


[failure] 11-11:
Trailing spaces not allowed

🤖 Prompt for AI Agents
In spec/AuthParserStandalone.spec.js around lines 11 to 120 there are multiple
trailing whitespace characters (notably around lines 11, 22, 24, 44, 46, 58, 66,
74, 78, 83) causing linter failures; remove the trailing spaces at the ends of
those lines (and any other trailing whitespace in the file) so no line ends with
extra spaces, then save the file (or run your project's formatter/linter
auto-fix like eslint --fix or prettier) to ensure the file passes linting.

@mtrezza
Copy link
Member

mtrezza commented Nov 7, 2025

Could you please make sure the tests use English language? They seem to be AI generated and quite verbose; could you make them more concise? I notice that only 1 of the added tests is failing, is that expected?

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