Skip to content

Fix GitHub Integration tests #20363

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Fix GitHub Integration tests #20363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
902448f
Fix GitHub Integration tests
filiptronicek Nov 13, 2024
978c0a6
Merge branch 'main' into ft/fix-github-tests
filiptronicek Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/server/src/dev/dev-data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export namespace DevData {
export function createGitHubTestToken(): Token {
if (!process.env.GITPOD_TEST_TOKEN_GITHUB) {
console.error(
`GITPOD_TEST_TOKEN_GITHUB env var is not set\n\n\t export GITPOD_TEST_TOKEN_GITHUB='{"username": "gitpod-test", "token": $AZURE_TOKEN}'`,
`GITPOD_TEST_TOKEN_GITHUB env var is not set\n\n\t export GITPOD_TEST_TOKEN_GITHUB='{"username": "gitpod-test", "value": $GITHUB_TOKEN}'`,
);
}
return {
Expand Down
2 changes: 1 addition & 1 deletion components/server/src/errors/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export namespace NotFoundError {
repoName: string,
errorMessage: string = "Repository not found.",
) {
const lastUpdate = (token && token.updateDate) || "";
const lastUpdate = (token && token.updateDate) ?? "";
const userScopes = token ? [...token.scopes] : [];

const userIsOwner = owner == user.name; // TODO: shouldn't this be a comparison with `identity.authName`?
Expand Down
7 changes: 4 additions & 3 deletions components/server/src/github/github-context-parser.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,12 @@ class TestGithubContextParser {

static readonly BLO_BLA_ERROR_DATA = {
host: "github.com",
lastUpdate: undefined,
lastUpdate: "",
owner: "blo",
repoName: "bla",
userIsOwner: false,
userScopes: ["user:email", "public_repo", "repo"],
errorMessage: "Could not resolve to a Repository with the name 'blo/bla'.",
};

protected getTestBranches(): BranchRef[] {
Expand Down Expand Up @@ -180,12 +181,12 @@ class TestGithubContextParser {
const result = await this.parser.handle(
{},
this.user,
"https://github.com/eclipse-theia/theia/tree/master/LICENSE",
Copy link
Member Author

Choose a reason for hiding this comment

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

The file disappeared, so we had to update it.

"https://github.com/eclipse-theia/theia/tree/master/LICENSE-EPL",
);
expect(result).to.deep.include({
ref: "master",
refType: "branch",
path: "LICENSE",
path: "LICENSE-EPL",
isFile: true,
repository: {
host: "github.com",
Expand Down
2 changes: 1 addition & 1 deletion components/server/src/github/github-repository-provider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class TestGithubContextRepositoryProvider {

@test public async testGetUserRepos() {
const result = await this.provider.getUserRepos(this.user);
expect(result).to.include({ url: "https://github.com/gitpod-io/gitpod", name: "gitpod" });
Copy link
Member Author

Choose a reason for hiding this comment

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

GitHub seemingly only considers recent contributions, who knew!

expect(result).to.deep.include({ url: "https://github.com/gitpod-integration-test/example", name: "example" });
}
}
module.exports = new TestGithubContextRepositoryProvider(); // Only to circumvent no usage warning :-/
7 changes: 4 additions & 3 deletions components/server/src/github/github-repository-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,19 +215,20 @@ export class GithubRepositoryProvider implements RepositoryProvider {
}`,
);

let repos: RepositoryInfo[] = [];

const repos: RepositoryInfo[] = [];
for (const type of ["contributedTo", "original", "forked"]) {
const nodes = result.data.viewer[type]?.nodes;
if (nodes) {
repos = nodes.map((n: any): RepositoryInfo => {
Copy link
Member Author

@filiptronicek filiptronicek Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This was actually a bug where we always rewrote the existing array if there were nodes for a type. Unlucky for us, even if there weren't, empty JS arrays are truthy, so the repos would get overridden anyway.

const reposInSection: RepositoryInfo[] = nodes.map((n: any) => {
return {
name: n.name,
url: n.url,
};
});
repos.push(...reposInSection);
}
}

return repos;
}

Expand Down
Loading