-
Notifications
You must be signed in to change notification settings - Fork 0
🧪 test: Add unit tests for initObservability #62
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
base: main
Are you sure you want to change the base?
Changes from all commits
3d07ade
efaef61
b31ace2
9049dbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| name: Auto Label | ||
| on: | ||
| pull_request: | ||
| types: [opened, reopened, synchronized] | ||
| jobs: | ||
| label: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const { data: files } = await github.rest.pulls.listFiles({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number: context.payload.pull_request.number | ||
| }); | ||
|
|
||
| const labels = new Set(); | ||
|
|
||
| for (const file of files) { | ||
| const path = file.filename; | ||
| if (path.includes('docs/') || path.endsWith('.md')) labels.add('documentation'); | ||
| if (path.includes('test/') || path.includes('.test.') || path.includes('.spec.')) labels.add('tests'); | ||
| if (path.includes('.github/workflows/')) labels.add('ci/cd'); | ||
| if (path.endsWith('.js') || path.endsWith('.ts') || path.endsWith('.jsx') || path.endsWith('.tsx')) labels.add('javascript'); | ||
| if (path.endsWith('.py')) labels.add('python'); | ||
| if (path.endsWith('.css') || path.endsWith('.scss') || path.endsWith('.sass')) labels.add('styling'); | ||
| } | ||
|
|
||
| if (labels.size > 0) { | ||
| await github.rest.issues.addLabels({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.payload.pull_request.number, | ||
| labels: Array.from(labels) | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| name: PR Checks | ||
| on: | ||
| pull_request: | ||
| types: [opened, reopened, synchronize, edited] | ||
| permissions: | ||
| issues: write | ||
| jobs: | ||
| validate: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const pr = context.payload.pull_request; | ||
| const issues = []; | ||
|
|
||
| if (pr.title.length < 10) { | ||
| issues.push('❌ PR title too short (minimum 10 characters)'); | ||
| } | ||
| if (!/^(feat|fix|docs|style|refactor|test|chore|perf|ci|build|revert)(\(.+\))?:/.test(pr.title)) { | ||
| issues.push('⚠️ PR title should follow conventional commits format'); | ||
| } | ||
|
|
||
| if (!pr.body || pr.body.length < 20) { | ||
| issues.push('❌ PR description is required (minimum 20 characters)'); | ||
| } | ||
|
|
||
| const totalChanges = (pr.additions || 0) + (pr.deletions || 0); | ||
| if (totalChanges > 500) { | ||
| issues.push(`⚠️ Large PR detected (${totalChanges} lines changed)`); | ||
| } | ||
|
|
||
| if (issues.length > 0) { | ||
| // Fork PRs get a read-only GITHUB_TOKEN; skip commenting to avoid errors | ||
| if (pr.head.repo.full_name === pr.base.repo.full_name) { | ||
| const comment = `## 🔍 PR Validation\n\n${issues.join('\n')}`; | ||
| await github.rest.issues.createComment({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: pr.number, | ||
| body: comment | ||
| }); | ||
| } else { | ||
| core.warning('Skipping PR comment for fork PR (read-only token)'); | ||
| issues.forEach(issue => core.warning(issue)); | ||
| } | ||
| core.setFailed('PR validation failed'); | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,91 @@ | |||||||||||||||||||||||
| import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; | |||||||||||||||||||||||
| import * as observabilityModule from './observability'; | |||||||||||||||||||||||
Check noticeCode scanning / CodeQL Unused variable, import, function or class Note
Unused import observabilityModule.
Copilot AutofixAI 4 days ago In general, unused imports should be removed to improve readability and avoid confusion. If the module is only needed for side effects, it should instead be imported without binding it to a name (e.g., The best fix without changing existing functionality is to delete the unused import line Concretely: in
Suggested changeset
1
packages/observability/src/observability.test.ts
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
|
|||||||||||||||||||||||
| import { Observability } from './observability'; | |||||||||||||||||||||||
Check noticeCode scanning / CodeQL Unused variable, import, function or class Note
Unused import Observability.
Copilot AutofixAI 4 days ago In general, unused imports should be removed to improve readability and avoid confusion. Since all uses of the The best fix here is to delete line 3 (
Suggested changeset
1
packages/observability/src/observability.test.ts
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
|
|||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| // Mock the open telemetry modules | |||||||||||||||||||||||
| vi.mock('@opentelemetry/sdk-node', () => { | |||||||||||||||||||||||
| return { | |||||||||||||||||||||||
| NodeSDK: vi.fn().mockImplementation(() => ({ | |||||||||||||||||||||||
| start: vi.fn(), | |||||||||||||||||||||||
| shutdown: vi.fn(), | |||||||||||||||||||||||
| })), | |||||||||||||||||||||||
| }; | |||||||||||||||||||||||
| }); | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| vi.mock('@opentelemetry/api', async () => { | |||||||||||||||||||||||
| const actual = await vi.importActual('@opentelemetry/api'); | |||||||||||||||||||||||
| return { | |||||||||||||||||||||||
| ...actual, | |||||||||||||||||||||||
| trace: { | |||||||||||||||||||||||
| getTracer: vi.fn().mockReturnValue({ | |||||||||||||||||||||||
| startActiveSpan: vi.fn((name, callback) => { | |||||||||||||||||||||||
| const mockSpan = { | |||||||||||||||||||||||
| setAttribute: vi.fn(), | |||||||||||||||||||||||
| setStatus: vi.fn(), | |||||||||||||||||||||||
| recordException: vi.fn(), | |||||||||||||||||||||||
| end: vi.fn(), | |||||||||||||||||||||||
| }; | |||||||||||||||||||||||
| return callback(mockSpan); | |||||||||||||||||||||||
| }), | |||||||||||||||||||||||
| }), | |||||||||||||||||||||||
| }, | |||||||||||||||||||||||
| metrics: { | |||||||||||||||||||||||
| getMeter: vi.fn().mockReturnValue({ | |||||||||||||||||||||||
| createCounter: vi.fn().mockReturnValue({ add: vi.fn() }), | |||||||||||||||||||||||
| createHistogram: vi.fn().mockReturnValue({ record: vi.fn() }), | |||||||||||||||||||||||
| }), | |||||||||||||||||||||||
| }, | |||||||||||||||||||||||
| }; | |||||||||||||||||||||||
| }); | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| describe('observability module exports', () => { | |||||||||||||||||||||||
| const testConfig = { | |||||||||||||||||||||||
| serviceName: 'test-service', | |||||||||||||||||||||||
| enabled: false // disabled to prevent actual connections | |||||||||||||||||||||||
| }; | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| beforeEach(() => { | |||||||||||||||||||||||
| // Reset vi modules so that each test gets a fresh internal observabilityInstance variable | |||||||||||||||||||||||
| vi.resetModules(); | |||||||||||||||||||||||
| vi.clearAllMocks(); | |||||||||||||||||||||||
| }); | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| describe('initObservability', () => { | |||||||||||||||||||||||
| it('should initialize and return an Observability instance', async () => { | |||||||||||||||||||||||
| // Import freshly to get a new instance | |||||||||||||||||||||||
| const mod = await import('./observability'); | |||||||||||||||||||||||
| const instance = mod.initObservability(testConfig); | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| expect(instance).toBeInstanceOf(mod.Observability); | |||||||||||||||||||||||
| // @ts-ignore - accessing private member for verification | |||||||||||||||||||||||
| expect(instance.serviceName).toBe('test-service'); | |||||||||||||||||||||||
| // @ts-ignore - accessing private member for verification | |||||||||||||||||||||||
| expect(instance.enabled).toBe(false); | |||||||||||||||||||||||
|
Comment on lines
+60
to
+63
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using A better approach is to test the observable behavior. For example, to test the References
Comment on lines
+60
to
+63
|
|||||||||||||||||||||||
| }); | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| it('should return the same instance when called multiple times', async () => { | |||||||||||||||||||||||
| const mod = await import('./observability'); | |||||||||||||||||||||||
| const instance1 = mod.initObservability(testConfig); | |||||||||||||||||||||||
| const instance2 = mod.initObservability({ serviceName: 'other-service' }); // Should ignore this config | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| expect(instance1).toBe(instance2); | |||||||||||||||||||||||
| // @ts-ignore | |||||||||||||||||||||||
| expect(instance2.serviceName).toBe('test-service'); | |||||||||||||||||||||||
|
Comment on lines
+72
to
+73
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As with the previous comment, it's best to avoid To verify that the singleton instance is not reconfigured on subsequent calls, you could check that the initialization logic (e.g., calls to References
|
|||||||||||||||||||||||
| }); | |||||||||||||||||||||||
| }); | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| describe('getObservability', () => { | |||||||||||||||||||||||
| it('should throw an error if called before initialization', async () => { | |||||||||||||||||||||||
| const mod = await import('./observability'); | |||||||||||||||||||||||
| expect(() => mod.getObservability()).toThrow('Observability not initialized. Call initObservability() first.'); | |||||||||||||||||||||||
| }); | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| it('should return the instance if already initialized', async () => { | |||||||||||||||||||||||
| const mod = await import('./observability'); | |||||||||||||||||||||||
| const initialized = mod.initObservability(testConfig); | |||||||||||||||||||||||
| const retrieved = mod.getObservability(); | |||||||||||||||||||||||
|
|
|||||||||||||||||||||||
| expect(retrieved).toBe(initialized); | |||||||||||||||||||||||
| }); | |||||||||||||||||||||||
| }); | |||||||||||||||||||||||
| }); | |||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| --- .github/workflows/auto-label.yml | ||
| +++ .github/workflows/auto-label.yml | ||
| @@ -3,6 +3,9 @@ | ||
| pull_request: | ||
| types: [opened, reopened, synchronized] | ||
| +permissions: | ||
| + issues: write | ||
| + pull-requests: write | ||
| jobs: | ||
| label: | ||
| runs-on: ubuntu-latest |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| --- .github/workflows/pr-checks.yml | ||
| +++ .github/workflows/pr-checks.yml | ||
| @@ -5,6 +5,7 @@ | ||
| types: [opened, reopened, synchronize, edited] | ||
| permissions: | ||
| issues: write | ||
| + pull-requests: write | ||
| jobs: | ||
| validate: | ||
| runs-on: ubuntu-latest |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Copilot Autofix
AI 4 days ago
In general, unused imports should be removed from the import list to avoid confusion and keep the codebase clean. Here, we only need to adjust the
vitestimport on line 1 to no longer includeafterEach.The best fix without changing functionality is to edit
packages/observability/src/observability.test.tsand removeafterEachfrom the destructuring import from'vitest'. No other lines or imports need to change, and no new methods or definitions are required. This keeps all currently used testing helpers (describe,it,expect,beforeEach,vi) while eliminating the unused one.