-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
test: Add failing test for PARSE_SERVER_AUTH_PROVIDERS env var (string vs object) #9909
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
Conversation
|
I will reformat the title to use the proper commit message syntax. |
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThree 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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.jsspec/parsers.spec.jsspec/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.jsspec/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.jsspec/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.jsspec/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.
spec/AuthParserStandalone.spec.js
Outdated
|
|
||
| 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'); | ||
| } | ||
| }); |
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.
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.
|
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? |
126e1ec to
f27b050
Compare
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_PROVIDERSis 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