Skip to content

Commit 026feb4

Browse files
cdcabreracdcabrera
committed
DEV refine
1 parent eeac65d commit 026feb4

File tree

4 files changed

+52
-219
lines changed

4 files changed

+52
-219
lines changed

‎jest.setupTests.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
// Shared helpers for all Jest tests
22

33
// Set NODE_ENV to test for all tests
4-
process.env.NODE_ENV = 'test';
4+
// process.env.NODE_ENV = 'test';

‎src/__tests__/__snapshots__/options.test.ts.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ exports[`options should return specific properties 1`] = `
8686
},
8787
},
8888
"URL_REGEX": /\\^\\(https\\?:\\)\\\\/\\\\//i,
89+
"generateSessionId": [Function],
8990
"parseCliOptions": [Function],
9091
"setOptions": [Function],
9192
}

‎src/__tests__/options.test.ts‎

Lines changed: 37 additions & 205 deletions
Original file line numberDiff line numberDiff line change
@@ -39,216 +39,48 @@ describe('parseCliOptions', () => {
3939
describe('setOptions', () => {
4040
it('should return options with consistent properties', () => {
4141
const result = setOptions({ docsHost: true });
42+
const { sessionId, ...remainingOptions } = result;
4243

43-
expect(Object.isFrozen(result)).toBe(false); // No longer frozen since we removed Object.freeze()
44-
expect(result).not.toBe(OPTIONS); // Now returns a fresh instance, not the global OPTIONS
45-
46-
// Verify sessionId is present and has correct format
47-
expect(result.sessionId).toBeDefined();
48-
expect(result.sessionId).toMatch(/^[a-f0-9]{40}$/); // SHA1 hash format
49-
50-
// Create a snapshot without the dynamic sessionId
51-
const { sessionId: _sessionId, ...resultWithoutSessionId } = result;
52-
53-
expect(resultWithoutSessionId).toMatchSnapshot('options');
44+
expect(result).not.toBe(OPTIONS);
45+
expect(sessionId).toBeDefined();
46+
expect(sessionId?.length).toBe(40);
47+
expect(remainingOptions).toMatchSnapshot('options');
5448
});
5549

56-
describe('OPTIONS cloning', () => {
57-
describe('structuredClone deep copying', () => {
58-
it('should create a deep copy of OPTIONS with unique sessionId', () => {
59-
const originalDocsHost = OPTIONS.docsHost;
60-
61-
// Create a fresh instance using setOptions
62-
const freshOptions = setOptions({ docsHost: !originalDocsHost });
63-
64-
// Verify sessionId is unique and present
65-
expect(freshOptions.sessionId).toBeDefined();
66-
expect(freshOptions.sessionId).toMatch(/^[a-f0-9]{40}$/); // SHA1 hash format
67-
expect(OPTIONS.sessionId).toBe(freshOptions.sessionId);
68-
69-
// Verify the values are correct
70-
expect(freshOptions.docsHost).toBe(!originalDocsHost);
71-
expect(OPTIONS.docsHost).toBe(!originalDocsHost);
72-
});
73-
74-
it('should create independent instances with unique sessionIds', () => {
75-
// Create first instance
76-
const firstOptions = setOptions({ docsHost: true });
77-
78-
expect(firstOptions.sessionId).toBeDefined();
79-
expect(firstOptions.docsHost).toBe(true);
80-
expect(OPTIONS.docsHost).toBe(true);
81-
82-
// Create second instance
83-
const secondOptions = setOptions({ docsHost: false });
84-
85-
expect(secondOptions.sessionId).toBeDefined();
86-
expect(secondOptions.docsHost).toBe(false);
87-
expect(OPTIONS.docsHost).toBe(false);
88-
89-
// Verify sessionIds are different (indicating fresh instances)
90-
expect(firstOptions.sessionId).not.toBe(secondOptions.sessionId);
91-
expect(OPTIONS.sessionId).toBe(secondOptions.sessionId);
92-
});
93-
94-
it('should handle nested object properties correctly', () => {
95-
const freshOptions = setOptions({ docsHost: true });
96-
97-
// Verify sessionId is present
98-
expect(freshOptions.sessionId).toBeDefined();
99-
100-
// Verify nested objects are properly cloned (deep clone)
101-
expect(freshOptions.resourceMemoOptions).toEqual(OPTIONS.resourceMemoOptions);
102-
expect(freshOptions.toolMemoOptions).toEqual(OPTIONS.toolMemoOptions);
103-
104-
// Note: structuredClone should create deep copies, but these objects might be shared
105-
// The important thing is that the sessionId is unique and the values are correct
106-
expect(freshOptions.resourceMemoOptions).toEqual(OPTIONS.resourceMemoOptions);
107-
expect(freshOptions.toolMemoOptions).toEqual(OPTIONS.toolMemoOptions);
108-
});
109-
110-
it('should preserve all OPTIONS properties including sessionId', () => {
111-
const freshOptions = setOptions({ docsHost: true });
112-
113-
// Verify sessionId is present
114-
expect(freshOptions.sessionId).toBeDefined();
115-
expect(freshOptions.sessionId).toMatch(/^[a-f0-9]{40}$/); // SHA1 hash format
116-
117-
// Verify all expected properties exist
118-
expect(freshOptions).toHaveProperty('pfExternal');
119-
expect(freshOptions).toHaveProperty('pfExternalCharts');
120-
expect(freshOptions).toHaveProperty('pfExternalChartsComponents');
121-
expect(freshOptions).toHaveProperty('pfExternalChartsDesign');
122-
expect(freshOptions).toHaveProperty('pfExternalDesign');
123-
expect(freshOptions).toHaveProperty('pfExternalDesignComponents');
124-
expect(freshOptions).toHaveProperty('pfExternalDesignLayouts');
125-
expect(freshOptions).toHaveProperty('pfExternalAccessibility');
126-
expect(freshOptions).toHaveProperty('resourceMemoOptions');
127-
expect(freshOptions).toHaveProperty('toolMemoOptions');
128-
expect(freshOptions).toHaveProperty('separator');
129-
expect(freshOptions).toHaveProperty('urlRegex');
130-
expect(freshOptions).toHaveProperty('name');
131-
expect(freshOptions).toHaveProperty('version');
132-
expect(freshOptions).toHaveProperty('repoName');
133-
expect(freshOptions).toHaveProperty('contextPath');
134-
expect(freshOptions).toHaveProperty('docsPath');
135-
expect(freshOptions).toHaveProperty('llmsFilesPath');
136-
expect(freshOptions).toHaveProperty('docsHost');
137-
expect(freshOptions).toHaveProperty('sessionId');
138-
});
139-
140-
it('should handle empty CLI options with sessionId', () => {
141-
const originalDocsHost = OPTIONS.docsHost;
142-
143-
const freshOptions = setOptions({});
144-
145-
// Verify sessionId is present
146-
expect(freshOptions.sessionId).toBeDefined();
147-
expect(OPTIONS.sessionId).toBe(freshOptions.sessionId);
148-
149-
// Verify docsHost remains unchanged
150-
expect(freshOptions.docsHost).toBe(originalDocsHost);
151-
expect(OPTIONS.docsHost).toBe(originalDocsHost);
152-
});
153-
154-
it('should handle partial CLI options with sessionId', () => {
155-
const originalName = OPTIONS.name;
156-
157-
const freshOptions = setOptions({ docsHost: true });
158-
159-
// Verify sessionId is present
160-
expect(freshOptions.sessionId).toBeDefined();
161-
expect(OPTIONS.sessionId).toBe(freshOptions.sessionId);
162-
163-
// Verify only docsHost changed
164-
expect(freshOptions.docsHost).toBe(true);
165-
expect(freshOptions.name).toBe(originalName);
166-
expect(OPTIONS.docsHost).toBe(true);
167-
expect(OPTIONS.name).toBe(originalName);
168-
});
169-
});
170-
171-
describe('OPTIONS immutability', () => {
172-
it('should not freeze the returned instance', () => {
173-
const freshOptions = setOptions({ docsHost: true });
174-
175-
// Verify sessionId is present
176-
expect(freshOptions.sessionId).toBeDefined();
177-
178-
// Verify the returned instance is not frozen
179-
expect(Object.isFrozen(freshOptions)).toBe(false);
180-
});
181-
182-
it('should allow modification of returned instance', () => {
183-
const freshOptions = setOptions({ docsHost: true });
184-
185-
// Verify sessionId is present
186-
expect(freshOptions.sessionId).toBeDefined();
187-
188-
// Should be able to modify the returned instance
189-
freshOptions.docsHost = false;
190-
expect(freshOptions.docsHost).toBe(false);
191-
192-
// OPTIONS should remain unchanged
193-
expect(OPTIONS.docsHost).toBe(true);
194-
});
195-
196-
it('should maintain isolation between instances using sessionId', () => {
197-
const firstOptions = setOptions({ docsHost: true });
198-
const secondOptions = setOptions({ docsHost: false });
199-
200-
// Verify sessionIds are different (indicating isolation)
201-
expect(firstOptions.sessionId).not.toBe(secondOptions.sessionId);
202-
expect(OPTIONS.sessionId).toBe(secondOptions.sessionId);
203-
204-
// Modify first instance
205-
firstOptions.docsHost = false;
206-
207-
// Second instance should be unaffected
208-
expect(secondOptions.docsHost).toBe(false);
209-
expect(OPTIONS.docsHost).toBe(false);
210-
});
211-
});
212-
213-
describe('multiple setOptions calls', () => {
214-
it('should handle multiple calls correctly with unique sessionIds', () => {
215-
const firstOptions = setOptions({ docsHost: true });
216-
217-
expect(firstOptions.sessionId).toBeDefined();
218-
expect(firstOptions.docsHost).toBe(true);
219-
expect(OPTIONS.docsHost).toBe(true);
220-
221-
const secondOptions = setOptions({ docsHost: false });
222-
223-
expect(secondOptions.sessionId).toBeDefined();
224-
expect(secondOptions.docsHost).toBe(false);
225-
expect(OPTIONS.docsHost).toBe(false);
226-
227-
const thirdOptions = setOptions({});
228-
229-
expect(thirdOptions.sessionId).toBeDefined();
230-
expect(thirdOptions.docsHost).toBe(false);
231-
expect(OPTIONS.docsHost).toBe(false);
232-
233-
// Verify all sessionIds are different
234-
expect(firstOptions.sessionId).not.toBe(secondOptions.sessionId);
235-
expect(secondOptions.sessionId).not.toBe(thirdOptions.sessionId);
236-
expect(firstOptions.sessionId).not.toBe(thirdOptions.sessionId);
237-
});
50+
it.each([{
51+
description: 'with docsHost set to true',
52+
firstOptions: { docsHost: true },
53+
secondOptions: { docsHost: true }
54+
},
55+
{
56+
description: 'with docsHost set differently',
57+
firstOptions: { docsHost: true },
58+
secondOptions: { docsHost: false }
59+
},
60+
{
61+
description: 'with empty options',
62+
firstOptions: {},
63+
secondOptions: {}
64+
}])('should create independent option instances, $description', ({ firstOptions, secondOptions }) => {
65+
// First instance
66+
const firstInstance = setOptions(firstOptions);
67+
68+
// Second instance
69+
const secondInstance = setOptions(secondOptions);
70+
71+
expect(OPTIONS).toEqual(secondInstance);
72+
expect(firstInstance).not.toEqual(secondInstance);
73+
expect(firstInstance.sessionId).not.toBe(secondInstance.sessionId);
74+
});
23875

239-
it('should create independent instances on each call with unique sessionIds', () => {
240-
const firstOptions = setOptions({ docsHost: true });
241-
const secondOptions = setOptions({ docsHost: false });
242-
const thirdOptions = setOptions({});
76+
it('should allow modification of returned options instance but not the global OPTIONS', () => {
77+
const freshOptions = setOptions({ docsHost: true });
24378

244-
// Verify all sessionIds are different (indicating fresh instances)
245-
expect(firstOptions.sessionId).not.toBe(secondOptions.sessionId);
246-
expect(secondOptions.sessionId).not.toBe(thirdOptions.sessionId);
247-
expect(firstOptions.sessionId).not.toBe(thirdOptions.sessionId);
79+
// Should be able to modify the returned instance
80+
freshOptions.docsHost = false;
81+
expect(freshOptions.docsHost).toBe(false);
24882

249-
// Verify OPTIONS has the latest sessionId
250-
expect(OPTIONS.sessionId).toBe(thirdOptions.sessionId);
251-
});
252-
});
83+
// OPTIONS should remain unchanged
84+
expect(OPTIONS.docsHost).toBe(true);
25385
});
25486
});

‎src/options.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -124,17 +124,6 @@ const PF_EXTERNAL_CHARTS_COMPONENTS = `${PF_EXTERNAL_CHARTS}/victory/components`
124124
*/
125125
const PF_EXTERNAL_CHARTS_DESIGN = `${PF_EXTERNAL_CHARTS}/charts`;
126126

127-
/**
128-
* Generate a unique session ID using hash-based approach
129-
*/
130-
const generateSessionId = (): string => {
131-
const timestamp = Date.now();
132-
const randomNumber = Math.floor(Math.random() * Number.MAX_SAFE_INTEGER);
133-
const data = `session_${timestamp}_${randomNumber}`;
134-
135-
return generateHash(data);
136-
};
137-
138127
/**
139128
* Global configuration options object.
140129
*
@@ -158,7 +147,7 @@ const generateSessionId = (): string => {
158147
* @property {string} contextPath - Current working directory.
159148
* @property {string} docsPath - Path to the documentation directory.
160149
* @property {string} llmsFilesPath - Path to the LLMs files directory.
161-
* @property {string} sessionId - Unique session identifier.
150+
* @property {string} [sessionId] - Unique session identifier.
162151
*/
163152
const OPTIONS: GlobalOptions = {
164153
pfExternal: PF_EXTERNAL,
@@ -179,7 +168,6 @@ const OPTIONS: GlobalOptions = {
179168
contextPath: (process.env.NODE_ENV === 'local' && '/') || process.cwd(),
180169
docsPath: (process.env.NODE_ENV === 'local' && '/documentation') || join(process.cwd(), 'documentation'),
181170
llmsFilesPath: (process.env.NODE_ENV === 'local' && '/llms-files') || join(process.cwd(), 'llms-files')
182-
// sessionId is optional and will be set when freezeOptions is called
183171
};
184172

185173
/**
@@ -190,6 +178,17 @@ const parseCliOptions = (): CliOptions => ({
190178
// Future CLI options can be added here
191179
});
192180

181+
/**
182+
* Generate a unique session ID
183+
*/
184+
const generateSessionId = (): string => {
185+
const timestamp = Date.now();
186+
const randomNumber = Math.floor(Math.random() * Number.MAX_SAFE_INTEGER);
187+
const data = `session_${timestamp}_${randomNumber}`;
188+
189+
return generateHash(data);
190+
};
191+
193192
/**
194193
* Make global options immutable after combining CLI options with app defaults.
195194
*
@@ -209,6 +208,7 @@ const setOptions = (cliOptions: CliOptions) => {
209208
};
210209

211210
export {
211+
generateSessionId,
212212
parseCliOptions,
213213
setOptions,
214214
OPTIONS,

0 commit comments

Comments
 (0)