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

Add support for codespaces #407

Merged
merged 90 commits into from
Aug 29, 2023

Conversation

vercellone
Copy link
Contributor

@vercellone vercellone commented Apr 29, 2023

Description

Add support for Codespaces.

Issues Fixed

References

Endpoints excluded below will be added in subsequent PRs

Codespaces/codespaces

Codespaces/organizations

Checklist

  • You actually ran the code that you just wrote, especially if you did just "one last quick change".
  • Comment-based help added/updated, including examples.
  • Static analysis is reporting back clean.
  • New/changed code adheres to our coding guidelines.
  • Formatters were created for any new types being added.
  • New/changed code continues to support the pipeline.
  • Changes to the manifest file follow the manifest guidance.
  • Unit tests were added/updated and are all passing. See testing guidelines. This includes making sure that all pipeline input variations have been covered.
  • Relevant usage examples have been added/updated in USAGE.md.
  • If desired, ensure your name is added to our Contributors list

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Thanks so much for this work, @vercellone!
This was in really great shape for a first contribution. I do have some feedback for you to address (please), but I don't anticipate it being a significant amount of work.

I'll admit that I had no idea that this GitHub feature even existed. I'm going to have to look to see what might be involved in getting a standard Codespace set up for this project (and some of my others).

GitHubCodespaces.ps1 Show resolved Hide resolved
GitHubCodespaces.ps1 Show resolved Hide resolved
GitHubCodespaces.ps1 Show resolved Hide resolved
GitHubCodespaces.ps1 Outdated Show resolved Hide resolved
GitHubCodespaces.ps1 Show resolved Hide resolved
GitHubCodespaces.ps1 Outdated Show resolved Hide resolved
GitHubCodespaces.ps1 Show resolved Hide resolved
GitHubCodespaces.ps1 Show resolved Hide resolved
GitHubCodespaces.ps1 Show resolved Hide resolved
GitHubCodespaces.ps1 Show resolved Hide resolved
@vercellone
Copy link
Contributor Author

Added

  • New-GitHubCodespace
  • Remove-GitHubCodespace

Keep the feedback coming. I'm particularly interested in your take on INPUTS, -Uri, -RepositoryId, and -PullRequest usage for New.

I will work on Tests next when able.

@vercellone
Copy link
Contributor Author

Good news
Repositories do not require any prep to create a codespace with New-GitHubCodespace. Of my repos, only vercellone/my-first-codespace includes .devcontainer/ config, and I can create viable codepaces for any of my repos. Given that, testing looks pretty straightforward.

Bad news
The url returned is always of the form https://api.github.com/user/codespaces/{codespace_name}, with no owner nor repo elements. Therefore, URI + Resolve-RepositoryElements isn't viable. I can figure something out, but suggestions are appreciated.

@HowardWolosky
Copy link
Member

Good news Repositories do not require any prep to create a codespace with New-GitHubCodespace. Of my repos, only vercellone/my-first-codespace includes .devcontainer/ config, and I can create viable codepaces for any of my repos. Given that, testing looks pretty straightforward.

Great!

Bad news The url returned is always of the form https://api.github.com/user/codespaces/{codespace_name}, with no owner nor repo elements. Therefore, URI + Resolve-RepositoryElements isn't viable. I can figure something out, but suggestions are appreciated.

At what point are you encountering this issue? In Add-GitHubCodespacesAdditionalProperties? If so, you have two options there:

  1. You can model something similar to what we do elsewhere where we think we may not have the necessary information in the response object -- just pass in the information we already know as an optional parameter. We do that for Repositories here:

[string] $OwnerName,
[string] $RepositoryName
)
foreach ($item in $InputObject)
{
$item.PSObject.TypeNames.Insert(0, $TypeName)
if (-not (Get-GitHubConfiguration -Name DisablePipelineSupport))
{
$repositoryUrl = [String]::Empty
if ([String]::IsNullOrEmpty($item.html_url))
{
if ($PSBoundParameters.ContainsKey('OwnerName') -and
$PSBoundParameters.ContainsKey('RepositoryName'))
{
$repositoryUrl = (Join-GitHubUri -OwnerName $OwnerName -RepositoryName $RepositoryName)
}
}
else
{
$elements = Split-GitHubUri -Uri $item.html_url
$repositoryUrl = Join-GitHubUri @elements
}
if (-not [String]::IsNullOrEmpty($repositoryUrl))
{
Add-Member -InputObject $item -Name 'RepositoryUrl' -Value $repositoryUrl -MemberType NoteProperty -Force
}

  1. The other option is to leverage the fact that a repository object is part of the response object and you're already sending it into Add-GitHubRepositoryAdditionalProperties, so you can just grab RepositoryUrl from the modified repository object afterwards and then add it to the root of your new object.

If you're encontering this issue elsewhere, then I'll need more context to know what and where the problem is.

@vercellone
Copy link
Contributor Author

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Looks like you still have a couple failing tests.

Tests/GitHubCodespaces.tests.ps1 Outdated Show resolved Hide resolved
Tests/GitHubCodespaces.tests.ps1 Show resolved Hide resolved
Tests/GitHubCodespaces.tests.ps1 Show resolved Hide resolved
@vercellone
Copy link
Contributor Author

Invoke-Pester -Configuration (
    [PesterConfiguration]@{
        Filter = @{ FullName = 'GitHubCodespaces\*-GitHubCodespace' }
        Output = @{ Verbosity = 'Detailed' }
        Path   = 'C:\Users\Jason\source\GitHub\PowerShellForGitHub\Tests\GitHubCodespaces.tests.ps1'
    }
)

Pester v5.4.1

Starting discovery in 28 files.
Discovery found 1128 tests in 616ms.
Filter 'FullName' set to ('GitHubCodespaces\*-GitHubCodespace').
Filters selected 30 tests to run.
Running tests.

Running tests from 'C:\Users\Jason\source\GitHub\PowerShellForGitHub\Tests\GitHubCodespaces.tests.ps1'
Describing GitHubCodespaces\Delete-GitHubCodespace
 Context When deleting a codespace for the authenticated user
   [+] Should get no content using -Confirm:$false 15.06s (15.06s|2ms)
   [+] Should get no content using -Force 15.02s (15.02s|1ms)

Describing GitHubCodespaces\Get-GitHubCodespace
 Context When getting codespaces for the authenticated user
   [+] Should return objects of the correct type 3ms (2ms|2ms)
   [+] Should return one or more results 3ms (2ms|1ms)
   [+] Should return the correct properties 2ms (2ms|1ms)
 Context When getting a codespace for a specified owner and repository
   [+] Should return objects of the correct type 3ms (2ms|1ms)
   [+] Should return one or more results 3ms (2ms|1ms)
   [+] Should return the correct properties 3ms (2ms|1ms)
 Context When getting a codespace for a specified organization user
   [+] Should have results for the organization user 4ms (3ms|1ms)
   [+] Should return the correct properties 3ms (2ms|1ms)
 Context When getting a codespace for a specified codespace name
   [+] Should return objects of the correct type 3ms (2ms|1ms)
   [+] Should return the correct properties 2ms (2ms|1ms)
 Context When specifiying the Uri parameter
   [+] Should return objects of the correct type 3ms (2ms|1ms)
   [+] Should return the correct properties 3ms (2ms|1ms)
 Context When specifiying the Uri parameter from the pipeline
   [+] Should return objects of the correct type 4ms (2ms|1ms)
   [+] Should return the correct properties 2ms (2ms|1ms)

Describing GitHubCodespaces\New-GitHubCodespace
 Context When creating a repository for the authenticated user
  Context When creating a codespace with default settings with RepositoryId
    [+] Should return an object of the correct type 10ms (6ms|3ms)
    [+] Should return the correct properties 4ms (3ms|1ms)
  Context When creating a codespace with default settings with Ref
    [+] Should return an object of the correct type 4ms (2ms|2ms)
    [+] Should return the correct properties 4ms (3ms|1ms)
  Context When creating a codespace with default settings from a PullRequest
    [+] Should return an object of the correct type 3ms (2ms|1ms)
    [+] Should return the correct properties 4ms (3ms|1ms)
  Context When creating a codespace with all possible settings
    [+] Should return an object of the correct type 3ms (2ms|1ms)
    [+] Should return the correct properties 7ms (7ms|1ms)
  Context When creating a codespace with default settings with Repository Elements
    [+] Should return an object of the correct type 3ms (2ms|1ms)
    [+] Should return the correct properties 4ms (3ms|1ms)

Describing GitHubCodespaces\Start-GitHubCodespace
 Context When starting a codespace for the authenticated user
   [+] Should not throw 4.26s (4.25s|2ms)
   [+] Should become Available 7.48s (7.48s|1ms)

Describing GitHubCodespaces\Stop-GitHubCodespace
 Context When stopping a codespace for the authenticated user
   [+] Should not throw 3.9s (3.9s|3ms)
   [+] Should become Shutdown 6.78s (6.78s|1ms)
Tests completed in 206.51s
Tests Passed: 30, Failed: 0, Skipped: 0 NotRun: 1098

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Still seeing two test failures:

[-] GitHubCodespaces\Get-GitHubCodespace.When getting codespaces for the authenticated user.Should return one or more results 44ms (42ms|2ms) Expected the actual value to be greater than or equal to 1, but got $null. at ($codespaces | Where-Object { $_ }).Count | Should -BeGreaterOrEqual 1, C:\Source\PowerShellForGitHub\tests\GitHubCodespaces.tests.ps1:88 at <ScriptBlock>, C:\Source\PowerShellForGitHub\tests\GitHubCodespaces.tests.ps1:88 [-] GitHubCodespaces\Get-GitHubCodespace.When getting a codespace for a specified owner and repository.Should return one or more results 12ms (11ms|1ms) Expected the actual value to be greater than or equal to 1, but got $null. at ($codespaces | Where-Object { $_ }).Count | Should -BeGreaterOrEqual 1, C:\Source\PowerShellForGitHub\tests\GitHubCodespaces.tests.ps1:110 at <ScriptBlock>, C:\Source\PowerShellForGitHub\tests\GitHubCodespaces.tests.ps1:110

@vercellone
Copy link
Contributor Author

Still seeing two test failures:

Thanks for the quick responses this week.

I could not reproduce these last 2 test failures in pwsh 7.3.6, but I was able to reproduce them in Windows PowerShell 5.1. They require an @ symbol before the ($codespaces...).Count to force the result to an Object[] array and avoid the InvokeMethodOnNull exception.

I think we should be good now.

@HowardWolosky
Copy link
Member

Can you also add last_used_at to this (will ensure that it gets converted to a real date/time object in the return object by default)?

# The list of property names across all of GitHub API v3 that are known to store dates as strings.
$script:datePropertyNames = @(
'closed_at',
'committed_at',
'completed_at',
'created_at',
'date',
'due_on',
'last_edited_at',
'last_read_at',
'merged_at',
'published_at',
'pushed_at',
'starred_at',
'started_at',
'submitted_at',
'timestamp',
'updated_at'
)

@vercellone
Copy link
Contributor Author

last_used_at added to datePropertyNames

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Thanks so much for all your hard work in getting this support added. Very much appreciated. Looking forward to hitting the button to merge this.

@HowardWolosky HowardWolosky merged commit 9ec863b into microsoft:master Aug 29, 2023
1 check passed
@vercellone
Copy link
Contributor Author

vercellone commented Aug 30, 2023

@vercellone vercellone deleted the feature/GitHubCodespaces branch August 30, 2023 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api completeness This is basic API functionality that hasn't been implemented yet. api-codespaces API support for Codespaces
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants