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

[CI] Failing Unit test at System package level #10860

Closed
wants to merge 13 commits into from

Conversation

singh1203
Copy link
Contributor

@singh1203 singh1203 commented May 3, 2024

Notes for Reviewers

This PR fixes #10758

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Saurabh Kumar Singh <[email protected]>
Signed-off-by: Saurabh Kumar Singh <[email protected]>
@github-actions github-actions bot added the component/mesheryctl CLI for Meshery label May 3, 2024
Copy link

github-actions bot commented May 3, 2024

Copy link

codecov bot commented May 3, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 9.33%. Comparing base (6179514) to head (1db8c3d).
Report is 3561 commits behind head on master.

Current head 1db8c3d differs from pull request most recent head 8ac58ef

Please upload reports for the commit 8ac58ef to get more accurate results.

Files Patch % Lines
mesheryctl/internal/cli/root/system/provider.go 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10860      +/-   ##
==========================================
- Coverage   10.13%    9.33%   -0.80%     
==========================================
  Files         140      146       +6     
  Lines       22170    19216    -2954     
==========================================
- Hits         2246     1794     -452     
+ Misses      19569    17091    -2478     
+ Partials      355      331      -24     
Flag Coverage Δ
unittests 9.33% <33.33%> (-0.80%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@singh1203 singh1203 marked this pull request as ready for review May 3, 2024 12:23
@singh1203 singh1203 changed the title [WIP] Failing Unit test at System package level #10851 [CI] Failing Unit test at System package level #10851 May 3, 2024
@singh1203 singh1203 changed the title [CI] Failing Unit test at System package level #10851 [CI] Failing Unit test at System package level May 3, 2024
@leecalcote leecalcote requested a review from lekaf974 May 4, 2024 03:41
@singh1203 singh1203 requested a review from lekaf974 May 7, 2024 05:26
Copy link
Member

@alphaX86 alphaX86 left a comment

Choose a reason for hiding this comment

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

Changes look good. Here are my queries I have...

mesheryctl/internal/cli/root/system/model.go Outdated Show resolved Hide resolved
mesheryctl/internal/cli/root/system/model_test.go Outdated Show resolved Hide resolved
@singh1203
Copy link
Contributor Author

singh1203 commented May 9, 2024

Changes look good. Here are my queries I have...

Please feel free to counter or point me in the right direction.
Thank you.

Copy link
Contributor

@MUzairS15 MUzairS15 left a comment

Choose a reason for hiding this comment

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

model has been elevated to its own standalone command. @singh1203
// @alphaX86 @RipulHandoo Correct?

@alphaX86
Copy link
Member

model has been elevated to its own standalone command. @singh1203
// @alphaX86 @RipulHandoo Correct?

It should be. But it's still present within system :/ so it'd become a complete refactor in this case

@singh1203
Copy link
Contributor Author

@alphaX86 Since it conflicts with the proper work that needs to be done, I believe that this PR is not necessary until this PR is merged #10602.
cc - @MUzairS15 @RipulHandoo

@alphaX86
Copy link
Member

@alphaX86 Since it conflicts with the proper work that needs to be done, I believe that this PR is not necessary until this PR is merged #10602. cc - @MUzairS15 @RipulHandoo

Seems that PR is auto-closed by the bot due to inactivity... // cc: @RipulHandoo

@singh1203
Copy link
Contributor Author

@alphaX86 Since it conflicts with the proper work that needs to be done, I believe that this PR is not necessary until this PR is merged #10602. cc - @MUzairS15 @RipulHandoo

Seems that PR is auto-closed by the bot due to inactivity... // cc: @RipulHandoo

Since PR #10602 is closed, I have removed conflicting changes and kept the ModelCmd as an exp command as previously for this PR continuity.

if len(args) == 0 {
return errors.New(utils.SystemProviderSubError("please specify a flag or subcommand. Use 'mesheryctl system provider --help' to display user guide.\n", "provider"))
}
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@leecalcote Here it is on L391

@leecalcote leecalcote requested review from Jougan-0 and removed request for leecalcote and MUzairS15 May 21, 2024 21:54
@Jougan-0
Copy link
Contributor

https://github.com/meshery/meshery/actions/runs/9137840475/job/25128309956?pr=10860
why failing merge recent branch and check i think my pr fixed it the mock server is not still running that is causing the issue and lee told to remove to remove the model_test.go completely here take a look

@singh1203
Copy link
Contributor Author

https://github.com/meshery/meshery/actions/runs/9137840475/job/25128309956?pr=10860
why failing merge recent branch and check i think my pr fixed it the mock server is not still running that is causing the issue and lee told to remove to remove the model_test.go completely here take a look

Due to some reason, will take a look after some time, resulting in an uneven 3-4 day time. Thank you!

@singh1203
Copy link
Contributor Author

Looks like your magic is working on this Parent PR #10703.
Thank you! @Jougan-0

https://github.com/meshery/meshery/actions/runs/9137840475/job/25128309956?pr=10860 why failing merge recent branch and check i think my pr fixed it the mock server is not still running that is causing the issue and lee told to remove to remove the model_test.go completely here take a look

@singh1203
Copy link
Contributor Author

Closing! It has already been resolved by #11002
// @leecalcote

@singh1203 singh1203 closed this May 22, 2024
@singh1203 singh1203 deleted the mesheryctl/test branch May 22, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/mesheryctl CLI for Meshery
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mesheryctl] Failing Unit test at System package level
7 participants