Skip to content
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

Playwright V2 E2E Framework #4706

Merged
merged 12 commits into from
Jul 3, 2024
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ out/
.env
.idea/
.run/
.test/
**/*.iml
**/*.vsix
index.scip
Expand Down
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
},
"editor.insertSpaces": true,
"cSpell.words": ["Supercompletion", "Supercompletions"],
"[json]": {
"editor.defaultFormatter": "biomejs.biome",
},
"[typescript]": {
"editor.defaultFormatter": "biomejs.biome"
},
Expand Down
4 changes: 2 additions & 2 deletions agent/src/cli/cody-bench/strategy-fix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { PromptString, ps } from '@sourcegraph/cody-shared'
import { glob } from 'glob'
import * as vscode from 'vscode'
import { ProtocolTextDocumentWithUri } from '../../../../vscode/src/jsonrpc/TextDocumentWithUri'
import { fileExists } from '../../../../vscode/src/local-context/download-symf'
import { pathExists } from '../../../../vscode/src/local-context/utils'
import { redactAuthorizationHeader } from '../../../../vscode/src/testutils/CodyPersister'
import { AgentTextDocument } from '../../AgentTextDocument'
import { TestClient } from '../../TestClient'
Expand Down Expand Up @@ -31,7 +31,7 @@ export async function evaluateFixStrategy(
token: options.srcAccessToken,
},
})
if (!(await fileExists(path.join(options.workspace, 'node_modules')))) {
if (!(await pathExists(path.join(options.workspace, 'node_modules')))) {
// Run pnpm install only when `node_modules` doesn't exist.
await runVoidCommand(options.installCommand, options.workspace)
}
Expand Down
4 changes: 1 addition & 3 deletions lib/shared/src/sourcegraph-api/graphql/url.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { trimEnd } from 'lodash'

const GRAPHQL_URI = '/.api/graphql'

interface BuildGraphQLUrlOptions {
Expand All @@ -12,5 +10,5 @@ interface BuildGraphQLUrlOptions {
export const buildGraphQLUrl = ({ request, baseUrl }: BuildGraphQLUrlOptions): string => {
const nameMatch = request ? request.match(/^\s*(?:query|mutation)\s+(\w+)/) : ''
const apiURL = `${GRAPHQL_URI}${nameMatch ? `?${nameMatch[1]}` : ''}`
return baseUrl ? new URL(trimEnd(baseUrl, '/') + apiURL).href : apiURL
return baseUrl ? new URL(apiURL, baseUrl).href : apiURL
}
Copy link
Member

Choose a reason for hiding this comment

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

FYI this doesn't mean the same as before since apiURL is an absolute path it will remove any path from baseUrl. What motivated this change? I do think this is safe though, since sourcegraph doesn't generally support not being the root of a domain.

> new URL('/foo', 'http://example.com/bar/').href
'http://example.com/foo'

22 changes: 22 additions & 0 deletions lib/shared/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,25 @@ export function createSubscriber<T>(): Subscriber<T> {
export function nextTick() {
return new Promise(resolve => process.nextTick(resolve))
}

export type SemverString<Prefix extends string> = `${Prefix}${number}.${number}.${number}`

export namespace SemverString {
const splitPrefixRegex = /^(?<prefix>.*)(?<version>\d+\.\d+\.\d+)$/
export function forcePrefix<P extends string>(prefix: P, value: string): SemverString<P> {
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure the version strings are always semver? What value do we have in enforcing this now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This type is used in the two locations where the only "guard" against mis-use was a comment left above it. Especially with the fact that one of them uses a 'v' and the other doesn't made that I had a very hard to debug issue as I simply misread the urls the whole time. This essentially just captures those two original comments as type-checks.

const match = splitPrefixRegex.exec(value)
if (!match || !match.groups?.version) {
throw new Error(`Invalid semver string: ${value}`)
}
return `${prefix}${match.groups?.version}` as SemverString<P>
}
}

type TupleFromUnion<T, U = T> = [T] extends [never]
? []
: T extends any
? [T, ...TupleFromUnion<Exclude<U, T>>]
: []

// Helper type to ensure an array contains all members of T
export type ArrayContainsAll<T extends string> = TupleFromUnion<T>
Loading
Loading