-
Notifications
You must be signed in to change notification settings - Fork 445
fix(init): fix handling of gitlab repos #7945
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
472e5ee
a2b7181
7e420b0
bcddb62
7fda9dd
ac45c4d
94c0146
6adbd47
ebc8b07
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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,120 @@ | ||
| import { describe, expect, it, vi } from 'vitest' | ||
| import type { RepoData } from '../../../src/utils/get-repo-data.js' | ||
|
|
||
| vi.mock('../../../src/utils/command-helpers.js', () => ({ | ||
| log: vi.fn(), | ||
| })) | ||
|
|
||
| describe('getRepoData', () => { | ||
| describe('RepoData structure for different Git providers', () => { | ||
| it('should construct correct httpsUrl for GitHub SSH URLs', () => { | ||
| const mockRepoData: RepoData = { | ||
| name: 'test', | ||
| owner: 'ownername', | ||
| repo: 'ownername/test', | ||
| url: 'git@github.com:ownername/test.git', | ||
| branch: 'main', | ||
| provider: 'github', | ||
| httpsUrl: 'https://github.com/ownername/test', | ||
| } | ||
|
|
||
| expect(mockRepoData.httpsUrl).toBe('https://github.com/ownername/test') | ||
| expect(mockRepoData.provider).toBe('github') | ||
| expect(mockRepoData.repo).toBe('ownername/test') | ||
| }) | ||
|
|
||
| it('should construct correct httpsUrl for GitLab SSH URLs', () => { | ||
| const mockRepoData: RepoData = { | ||
| name: 'test', | ||
| owner: 'ownername', | ||
| repo: 'ownername/test', | ||
| url: 'git@gitlab.com:ownername/test.git', | ||
| branch: 'main', | ||
| provider: 'gitlab', | ||
| httpsUrl: 'https://gitlab.com/ownername/test', | ||
| } | ||
|
|
||
| expect(mockRepoData.httpsUrl).toBe('https://gitlab.com/ownername/test') | ||
| expect(mockRepoData.provider).toBe('gitlab') | ||
| expect(mockRepoData.repo).toBe('ownername/test') | ||
| }) | ||
|
|
||
| it('should construct correct httpsUrl for GitHub HTTPS URLs', () => { | ||
| const mockRepoData: RepoData = { | ||
| name: 'test', | ||
| owner: 'ownername', | ||
| repo: 'ownername/test', | ||
| url: 'https://github.com/ownername/test.git', | ||
| branch: 'main', | ||
| provider: 'github', | ||
| httpsUrl: 'https://github.com/ownername/test', | ||
| } | ||
|
|
||
| expect(mockRepoData.httpsUrl).toBe('https://github.com/ownername/test') | ||
| expect(mockRepoData.provider).toBe('github') | ||
| expect(mockRepoData.repo).toBe('ownername/test') | ||
| }) | ||
|
|
||
| it('should construct correct httpsUrl for GitLab HTTPS URLs', () => { | ||
| const mockRepoData: RepoData = { | ||
| name: 'test', | ||
| owner: 'ownername', | ||
| repo: 'ownername/test', | ||
| url: 'https://gitlab.com/ownername/test.git', | ||
| branch: 'main', | ||
| provider: 'gitlab', | ||
| httpsUrl: 'https://gitlab.com/ownername/test', | ||
| } | ||
|
|
||
| expect(mockRepoData.httpsUrl).toBe('https://gitlab.com/ownername/test') | ||
| expect(mockRepoData.provider).toBe('gitlab') | ||
| expect(mockRepoData.repo).toBe('ownername/test') | ||
| }) | ||
|
|
||
| it('should use host as provider for unknown Git hosts', () => { | ||
| const mockRepoData: RepoData = { | ||
| name: 'test', | ||
| owner: 'user', | ||
| repo: 'user/test', | ||
| url: 'git@custom-git.example.com:user/test.git', | ||
| branch: 'main', | ||
| provider: 'custom-git.example.com', | ||
| httpsUrl: 'https://custom-git.example.com/user/test', | ||
| } | ||
|
|
||
| expect(mockRepoData.httpsUrl).toBe('https://custom-git.example.com/user/test') | ||
| expect(mockRepoData.provider).toBe('custom-git.example.com') | ||
| expect(mockRepoData.repo).toBe('user/test') | ||
| }) | ||
| }) | ||
|
|
||
| describe('provider field mapping', () => { | ||
| it('should map github.com to "github" provider', () => { | ||
| const mockRepoData: RepoData = { | ||
| name: 'test', | ||
| owner: 'user', | ||
| repo: 'user/test', | ||
| url: 'git@github.com:user/test.git', | ||
| branch: 'main', | ||
| provider: 'github', | ||
| httpsUrl: 'https://github.com/user/test', | ||
| } | ||
|
|
||
| expect(mockRepoData.provider).toBe('github') | ||
| }) | ||
|
|
||
| it('should map gitlab.com to "gitlab" provider', () => { | ||
| const mockRepoData: RepoData = { | ||
| name: 'test', | ||
| owner: 'user', | ||
| repo: 'user/test', | ||
| url: 'git@gitlab.com:user/test.git', | ||
| branch: 'main', | ||
| provider: 'gitlab', | ||
| httpsUrl: 'https://gitlab.com/user/test', | ||
| } | ||
|
|
||
| expect(mockRepoData.provider).toBe('gitlab') | ||
| }) | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,167 @@ | ||||||||||||||||||||||
| import { describe, expect, it, vi, beforeEach } from 'vitest' | ||||||||||||||||||||||
| import type { RepoData } from '../../../../src/utils/get-repo-data.js' | ||||||||||||||||||||||
| import type { NetlifyAPI } from '@netlify/api' | ||||||||||||||||||||||
| import type BaseCommand from '../../../../src/commands/base-command.js' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const mockPrompt = vi.fn() | ||||||||||||||||||||||
| const mockLog = vi.fn() | ||||||||||||||||||||||
| const mockExit = vi.fn() | ||||||||||||||||||||||
| const mockCreateDeployKey = vi.fn() | ||||||||||||||||||||||
| const mockGetBuildSettings = vi.fn() | ||||||||||||||||||||||
| const mockSaveNetlifyToml = vi.fn() | ||||||||||||||||||||||
| const mockSetupSite = vi.fn() | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| vi.mock('inquirer', () => ({ | ||||||||||||||||||||||
| default: { | ||||||||||||||||||||||
| prompt: mockPrompt, | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| })) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| vi.mock('../../../../src/utils/command-helpers.js', () => ({ | ||||||||||||||||||||||
| log: mockLog, | ||||||||||||||||||||||
| exit: mockExit, | ||||||||||||||||||||||
| })) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| vi.mock('../../../../src/utils/init/utils.js', () => ({ | ||||||||||||||||||||||
| createDeployKey: mockCreateDeployKey, | ||||||||||||||||||||||
| getBuildSettings: mockGetBuildSettings, | ||||||||||||||||||||||
| saveNetlifyToml: mockSaveNetlifyToml, | ||||||||||||||||||||||
| setupSite: mockSetupSite, | ||||||||||||||||||||||
| })) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| describe('config-manual', () => { | ||||||||||||||||||||||
| let mockApi: Partial<NetlifyAPI> | ||||||||||||||||||||||
| let mockCommand: Pick<BaseCommand, 'netlify'> | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| beforeEach(() => { | ||||||||||||||||||||||
| vi.clearAllMocks() | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| mockApi = {} | ||||||||||||||||||||||
| mockCommand = { | ||||||||||||||||||||||
| netlify: { | ||||||||||||||||||||||
| api: mockApi as NetlifyAPI, | ||||||||||||||||||||||
| cachedConfig: { configPath: '/test/netlify.toml' } as BaseCommand['netlify']['cachedConfig'], | ||||||||||||||||||||||
| config: { plugins: [] } as BaseCommand['netlify']['config'], | ||||||||||||||||||||||
|
Check failure on line 44 in tests/unit/utils/init/config-manual.test.ts
|
||||||||||||||||||||||
| repositoryRoot: '/test', | ||||||||||||||||||||||
|
Comment on lines
+41
to
+45
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. π§© Analysis chainπ Script executed: git ls-files | grep -E "(config-manual\.test\.ts|base-command\.ts)"Repository: netlify/cli Length of output: 127 π Script executed: cat -n tests/unit/utils/init/config-manual.test.ts | sed -n '35,50p'Repository: netlify/cli Length of output: 608 π Script executed: head -100 src/commands/base-command.ts | cat -nRepository: netlify/cli Length of output: 4256 π Script executed: cat -n src/commands/base-command.ts | sed -n '100,250p'Repository: netlify/cli Length of output: 6552 π Script executed: cat -n src/commands/types.tsRepository: netlify/cli Length of output: 108 π Script executed: rg -t ts "type NetlifyOptions|interface NetlifyOptions" --max-count 5Repository: netlify/cli Length of output: 109 π Script executed: cat -n src/commands/types.d.tsRepository: netlify/cli Length of output: 1910 π Script executed: rg -t ts "type NormalizedCachedConfigConfig|interface NormalizedCachedConfigConfig" -A 15Repository: netlify/cli Length of output: 14867 π Script executed: rg -t ts "type CachedConfig|interface CachedConfig" src/lib/build.js -A 20Repository: netlify/cli Length of output: 111 π Script executed: fd build src/lib -t f | head -10Repository: netlify/cli Length of output: 71 π Script executed: rg "type CachedConfig|interface CachedConfig" --max-count 5Repository: netlify/cli Length of output: 154 π Script executed: rg "export interface CachedConfig" -A 30 src/lib/build.tsRepository: netlify/cli Length of output: 1068 π Script executed: rg "export interface CachedConfig" -A 60 src/lib/build.ts | head -80Repository: netlify/cli Length of output: 2080 π Script executed: rg "export interface CachedConfig" -A 150 src/lib/build.ts | grep -A 5 "plugins"Repository: netlify/cli Length of output: 499 π Script executed: cat -n tests/unit/utils/init/config-manual.test.ts | head -80Repository: netlify/cli Length of output: 3052 π Script executed: cat tsconfig.json | head -50Repository: netlify/cli Length of output: 94 π Script executed: cat tsconfig.base.json | head -80Repository: netlify/cli Length of output: 403 π Script executed: cd /tmp && cat > check_types.ts << 'EOF'
import type { CachedConfig } from '/repo/src/lib/build'
import type BaseCommand from '/repo/src/commands/base-command'
// This is what the test is trying to do
const testConfig: BaseCommand['netlify']['config'] = { plugins: [] }
EOF
npx tsc --noEmit check_types.ts 2>&1Repository: netlify/cli Length of output: 300 π Script executed: rg "export interface CachedConfig" -A 200 src/lib/build.ts | tail -50Repository: netlify/cli Length of output: 1462 Fix the failing type assertion on Line 44 attempts an invalid direct cast. The Proposed fix- config: { plugins: [] } as BaseCommand['netlify']['config'],
+ config: { plugins: [] } as unknown as BaseCommand['netlify']['config'],π Committable suggestion
Suggested change
π§° Toolsπͺ GitHub Actions: Typecheck[error] 44-44: TypeScript error TS2352: Conversion of type '{ plugins: never[]; }' to type 'NormalizedCachedConfigConfig' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first. (During npm run typecheck/tsc) πͺ GitHub Check: typecheck[failure] 44-44: π€ Prompt for AI Agents |
||||||||||||||||||||||
| } as BaseCommand['netlify'], | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| mockPrompt.mockResolvedValue({ | ||||||||||||||||||||||
| sshKeyAdded: true, | ||||||||||||||||||||||
| repoPath: 'git@gitlab.com:test/repo.git', | ||||||||||||||||||||||
| deployHookAdded: true, | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| mockCreateDeployKey.mockResolvedValue({ id: 'key-123', public_key: 'ssh-rsa test' }) | ||||||||||||||||||||||
| mockGetBuildSettings.mockResolvedValue({ | ||||||||||||||||||||||
| baseDir: '', | ||||||||||||||||||||||
| buildCmd: 'npm run build', | ||||||||||||||||||||||
| buildDir: 'dist', | ||||||||||||||||||||||
| functionsDir: 'functions', | ||||||||||||||||||||||
| pluginsToInstall: [], | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
| mockSaveNetlifyToml.mockResolvedValue(undefined) | ||||||||||||||||||||||
| mockSetupSite.mockResolvedValue({ deploy_hook: 'https://api.netlify.com/hooks/test' }) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| describe('GitLab repository configuration', () => { | ||||||||||||||||||||||
| it('should use provider from repoData for GitLab repos', async () => { | ||||||||||||||||||||||
| const configManual = (await import('../../../../src/utils/init/config-manual.js')).default | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const repoData: RepoData = { | ||||||||||||||||||||||
| name: 'test', | ||||||||||||||||||||||
| owner: 'ownername', | ||||||||||||||||||||||
| repo: 'ownername/test', | ||||||||||||||||||||||
| url: 'git@gitlab.com:ownername/test.git', | ||||||||||||||||||||||
| branch: 'main', | ||||||||||||||||||||||
| provider: 'gitlab', | ||||||||||||||||||||||
| httpsUrl: 'https://gitlab.com/ownername/test', | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| await configManual({ | ||||||||||||||||||||||
| command: mockCommand as BaseCommand, | ||||||||||||||||||||||
| repoData, | ||||||||||||||||||||||
| siteId: 'site-123', | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const setupCall = mockSetupSite.mock.calls[0][0] as { repo: { provider: string; repo_path: string } } | ||||||||||||||||||||||
| expect(setupCall.repo.provider).toBe('gitlab') | ||||||||||||||||||||||
| expect(setupCall.repo.repo_path).toBe('ownername/test') | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| it('should use repo path (owner/name format) instead of SSH URL for GitLab', async () => { | ||||||||||||||||||||||
| const configManual = (await import('../../../../src/utils/init/config-manual.js')).default | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const repoData: RepoData = { | ||||||||||||||||||||||
| name: 'test', | ||||||||||||||||||||||
| owner: 'ownername', | ||||||||||||||||||||||
| repo: 'ownername/test', | ||||||||||||||||||||||
| url: 'git@gitlab.com:ownername/test.git', | ||||||||||||||||||||||
| branch: 'main', | ||||||||||||||||||||||
| provider: 'gitlab', | ||||||||||||||||||||||
| httpsUrl: 'https://gitlab.com/ownername/test', | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| await configManual({ | ||||||||||||||||||||||
| command: mockCommand as BaseCommand, | ||||||||||||||||||||||
| repoData, | ||||||||||||||||||||||
| siteId: 'site-123', | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const setupSiteCall = mockSetupSite.mock.calls[0][0] as { | ||||||||||||||||||||||
| repo: { repo_path: string } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| expect(setupSiteCall.repo.repo_path).toBe('ownername/test') | ||||||||||||||||||||||
| expect(setupSiteCall.repo.repo_path).not.toBe('git@gitlab.com:ownername/test.git') | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| it('should fallback to manual provider when provider is null', async () => { | ||||||||||||||||||||||
| const configManual = (await import('../../../../src/utils/init/config-manual.js')).default | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const repoData: RepoData = { | ||||||||||||||||||||||
| name: 'test', | ||||||||||||||||||||||
| owner: 'user', | ||||||||||||||||||||||
| repo: 'user/test', | ||||||||||||||||||||||
| url: 'git@custom.com:user/test.git', | ||||||||||||||||||||||
| branch: 'main', | ||||||||||||||||||||||
| provider: null, | ||||||||||||||||||||||
| httpsUrl: 'https://custom.com/user/test', | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| await configManual({ | ||||||||||||||||||||||
| command: mockCommand as BaseCommand, | ||||||||||||||||||||||
| repoData, | ||||||||||||||||||||||
| siteId: 'site-123', | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const setupCall = mockSetupSite.mock.calls[0][0] as { repo: { provider: string } } | ||||||||||||||||||||||
| expect(setupCall.repo.provider).toBe('manual') | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| describe('GitHub repository configuration', () => { | ||||||||||||||||||||||
| it('should use provider from repoData for GitHub repos', async () => { | ||||||||||||||||||||||
| const configManual = (await import('../../../../src/utils/init/config-manual.js')).default | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const repoData: RepoData = { | ||||||||||||||||||||||
| name: 'test', | ||||||||||||||||||||||
| owner: 'user', | ||||||||||||||||||||||
| repo: 'user/test', | ||||||||||||||||||||||
| url: 'git@github.com:user/test.git', | ||||||||||||||||||||||
| branch: 'main', | ||||||||||||||||||||||
| provider: 'github', | ||||||||||||||||||||||
| httpsUrl: 'https://github.com/user/test', | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| await configManual({ | ||||||||||||||||||||||
| command: mockCommand as BaseCommand, | ||||||||||||||||||||||
| repoData, | ||||||||||||||||||||||
| siteId: 'site-123', | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const setupCall = mockSetupSite.mock.calls[0][0] as { repo: { provider: string; repo_path: string } } | ||||||||||||||||||||||
| expect(setupCall.repo.provider).toBe('github') | ||||||||||||||||||||||
| expect(setupCall.repo.repo_path).toBe('user/test') | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
| }) | ||||||||||||||||||||||
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.
Tests are self-fulfilling; exercise
getRepoDatainstead.These cases construct
RepoDataliterals and assert the same values, so they donβt validate parsing/normalization behavior. Consider driving the tests throughgetRepoData(with mocked repo info) and asserting its output to catch regressions.π€ Prompt for AI Agents