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

feat(pack): add support for scaffolding new extensions #2026

Closed
wants to merge 15 commits into from

Conversation

sarthaksarthak9
Copy link
Contributor

@sarthaksarthak9 sarthaksarthak9 commented Jan 17, 2024

add support for scaffolding new extensions

Summary

add support for scaffolding new extensions

Output

Before

After

Screenshot from 2024-01-17 21-26-07

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

Resolves #1636

add support for scaffolding new extensions
@sarthaksarthak9 sarthaksarthak9 requested review from a team as code owners January 17, 2024 16:11
@github-actions github-actions bot added this to the 0.33.0 milestone Jan 17, 2024
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Jan 17, 2024
@sarthaksarthak9 sarthaksarthak9 marked this pull request as draft January 17, 2024 16:16
@sarthaksarthak9 sarthaksarthak9 marked this pull request as ready for review January 17, 2024 16:26
@sarthaksarthak9
Copy link
Contributor Author

@natalieparellano could you plz review the pr ?

@sarthaksarthak9
Copy link
Contributor Author

@natalieparellano I'm working on test for newExtension. Do we have anything similar to newBuildpack() method https://pkg.go.dev/github.com/buildpacks/pack/internal/commands/testmocks#MockPackClientMockRecorder.NewBuildpack for extension ?

@jjbustamante jjbustamante modified the milestones: 0.33.0, 0.34.0 Jan 23, 2024
Implement NewExtension method in MockPackClient
@github-actions github-actions bot modified the milestones: 0.34.0, 0.33.0 Jan 24, 2024
@sarthaksarthak9
Copy link
Contributor Author

sarthaksarthak9 commented Jan 24, 2024

I run make build, make format, make tidy and make verify they are running good at my end !! I don't know y these tests are failing !!

@jjbustamante
Copy link
Member

jjbustamante commented Jan 24, 2024

I run make build make format make tidy and make verify they are running good at my end !! I don't know y these tests are failing !!

Hi @sarthaksarthak9

For what I can see, it looks like your mock setup is missing some information

 Got: {0.8 /tmp/build-test1320509439/targets example/targets 1.0.0 [] [{linux arm v6 [{ubuntu [14.04 16.04]}]}]} (client.NewBuildpackOptions)

  Want: is equal to {0.8 /tmp/build-test1320509439/targets example/targets 1.0.0} (client.NewExtensionOptions)

Your expected object is missing [] [{linux arm v6 [{ubuntu [14.04 16.04]}]}] these attributes.

Maybe here:

mockClient.EXPECT().NewBuildpack(gomock.Any(), client.NewExtensionOptions{
					API:     "0.8",
					ID:      "example/targets",
					Path:    filepath.Join(tmpDir, "targets"),
					Version: "1.0.0",
				}).Return(nil).MaxTimes(1)

You need to add more details on the targets options

@jjbustamante jjbustamante modified the milestones: 0.33.0, 0.34.0 Jan 24, 2024
@sarthaksarthak9
Copy link
Contributor Author

sarthaksarthak9 commented Jan 25, 2024

@jjbustamante targets are not present in ExtensionDescriptor struct and ExtensionNewFlags, do I have to add it ? https://github.com/buildpacks/pack/blob/main/pkg/dist/extension_descriptor.go

enable target support for extension
@github-actions github-actions bot modified the milestones: 0.34.0, 0.33.0 Jan 25, 2024
@jjbustamante jjbustamante modified the milestones: 0.33.0, 0.34.0 Jan 25, 2024
@sarthaksarthak9
Copy link
Contributor Author

Screenshot from 2024-01-27 10-15-31
@natalieparellano @jjbustamante I think we have testDeepContains as last unit test, still Make file throwing an error!! Could you pls check this out!!

@jjbustamante
Copy link
Member

Hi @sarthaksarthak9

I think the problem is that your tests are still not passing. I cloned your branch and try to fix them. This is what I changed:

Screenshot 2024-01-29 at 10 55 52 AM

I think you copied and pasted some tests from a previous command, which is ok, but you didn't changed the actual method you wanna mock NewExtension. The other error seems to be with the API version.

I did those changes and the tests started passing locally

@sarthaksarthak9
Copy link
Contributor Author

sarthaksarthak9 commented Jan 29, 2024

Hi @sarthaksarthak9

I think the problem is that your tests are still not passing. I cloned your branch and try to fix them. This is what I changed:

Screenshot 2024-01-29 at 10 55 52 AM

I think you copied and pasted some tests from a previous command, which is ok, but you didn't changed the actual method you wanna mock NewExtension. The other error seems to be with the API version.

I did those changes and the tests started passing locally

This is extremely helpful. I completely missed that it needed updating. Thank you for catching that!

@github-actions github-actions bot modified the milestones: 0.34.0, 0.33.0 Jan 29, 2024
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

Attention: Patch coverage is 29.16667% with 102 lines in your changes missing coverage. Please review.

Project coverage is 78.42%. Comparing base (e3f8bc2) to head (63de73b).
Report is 209 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2026      +/-   ##
==========================================
- Coverage   79.63%   78.42%   -1.21%     
==========================================
  Files         176      177       +1     
  Lines       13214    13352     +138     
==========================================
- Hits        10522    10470      -52     
- Misses       2022     2213     +191     
+ Partials      670      669       -1     
Flag Coverage Δ
os_linux 78.04% <29.17%> (-0.51%) ⬇️
os_macos 75.90% <29.17%> (-0.49%) ⬇️
os_windows 77.77% <29.17%> (-1.25%) ⬇️
unit 78.42% <29.17%> (-1.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@sarthaksarthak9
Copy link
Contributor Author

I commits the changes with wrong author, due to which DCO is not passing because of this I am closing this pull request and opening a new

@sarthaksarthak9 sarthaksarthak9 deleted the scafall branch January 30, 2024 04:13
@sarthaksarthak9 sarthaksarthak9 restored the scafall branch January 30, 2024 15:32
@jjbustamante jjbustamante removed this from the 0.33.0 milestone Feb 1, 2024
@sarthaksarthak9 sarthaksarthak9 deleted the scafall branch April 21, 2024 04:35
@sarthaksarthak9 sarthaksarthak9 restored the scafall branch June 30, 2024 08:38
@github-actions github-actions bot added this to the 0.35.0 milestone Jun 30, 2024
@sarthaksarthak9 sarthaksarthak9 deleted the scafall branch June 30, 2024 08:39
@jjbustamante jjbustamante removed this from the 0.35.0 milestone Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pack should support pack extension new
3 participants