cli icon indicating copy to clipboard operation
cli copied to clipboard

Get to ginkgo v2

Open moleske opened this issue 2 years ago • 8 comments

Update - panics all gone, one failing test as described in this comment (below here) that I need help resolving. Everything else seems to be working. I think future work would need to give thought if it is worth trying to backport this at all to v8 and/or v7

old original description details

  • panic occuring because of mismatch ginkgo versions due to ginkgo v1 being needed by code.cloudfoundry.org/cfnetworking-cli-api
  • code.cloudfoundry.org/cfnetworking-cli-api is archived and hasn't been maintained for quite awhile
  • the error shows as /Users/moleske/workspace/cli/util/clissh/clissh.test flag redefined: ginkgo.seed panic: /Users/moleske/workspace/cli/util/clissh/clissh.test flag redefined: ginkgo.seed

goroutine 1 [running]: flag.(*FlagSet).Var(0x14000116120, {0x1049bcdf8, 0x104c3e280}, {0x1400011d020, 0xb}, {0x104842d23, 0x2a}) /opt/homebrew/Cellar/go/1.20.1/libexec/src/flag/flag.go:982 +0x2a4 flag.(*FlagSet).Int64Var(...) /opt/homebrew/Cellar/go/1.20.1/libexec/src/flag/flag.go:769 github.com/onsi/ginkgo/config.Flags(0x1049370c0?, {0x10482abdb?, 0x104843acb?}, 0x1) /Users/moleske/go/pkg/mod/github.com/onsi/[email protected]/config/config.go:75 +0xe4 github.com/onsi/ginkgo.init.0() /Users/moleske/go/pkg/mod/github.com/onsi/[email protected]/ginkgo_dsl.go:53 +0x34

Ginkgo ran 20 suites in 10.659707541s Test Suite Failed make: *** [units-non-plugin] Error 1

cc @cloudfoundry/wg-app-runtime-interfaces-cli-approvers if you all have thoughts Thank you for contributing to the CF CLI! Please read the following:

  • Please make sure you have implemented changes in line with the contributing guidelines
  • We're not allowed to accept any PRs without a signed CLA, no matter how small. If your contribution falls under a company CLA but your membership is not public, expect delays while we confirm.
  • All new code requires tests to protect against regressions.
  • Contributions must be made against the appropriate branch. See the contributing guidelines
  • Contributions must conform to our style guide. Please reach out to us if you have questions.

Does this PR modify CLI v6, CLI v7, or CLI v8?

nah Please see the contribution doc above or review Architecture Guide.

Description of the Change

get to ginkgo v2 hotness

We must be able to understand the design of your change from this description. Keep in mind that the maintainer reviewing this PR may not be familiar with or have worked with the code here recently, so please walk us through the concepts.

Why Is This PR Valuable?

What benefits will be realized by the code change? What users would want this change? What user need is this change addressing? cause ginkgo v1 not supported no more

Why Should This Be In Core?

Explain why this functionality should be in the cf CLI, as opposed to a plugin.

Applicable Issues

List any applicable GitHub Issues here

How Urgent Is The Change?

Is the change urgent? If so, explain why it is time-sensitive.

Other Relevant Parties

Who else is affected by the change?

moleske avatar Feb 25 '23 00:02 moleske

if you all have thoughts on proper way to remove code.cloudfoundry.org/cfnetworking-cli-api as a dependency give a holler

moleske avatar Feb 25 '23 00:02 moleske

@moleske this is a great initiative - V2 brings new tools to navigate flakes. @ccjaimes and @pivotalgeorge might be interested in this too

a-b avatar Aug 15 '23 21:08 a-b

reading the commit history of the archive repo that we depend on (https://github.com/cloudfoundry-incubator/cfnetworking-cli-api/commits/master) that is causing problems here - it seems like that repo was created by ripping it out of the cli code initially. Seems reasonable to me to just bring it back if we're not maintaining it in that archived repo no longer

moleske avatar Sep 15 '23 22:09 moleske

apparently same thing happened with https://github.com/cloudfoundry/gofileutils/commits/master

moleske avatar Sep 15 '23 22:09 moleske

We have a bunch of green units by disabling parallel execution.

a-b avatar Feb 01 '24 20:02 a-b

CLA Not Signed

  • :white_check_mark: login: a-b / name: Al Berez (b4a86fa3e3ec95e3a1b421562e773ef8e4d34d87)
  • :x: - login: @moleske / name: M. Oleske . The commit (5b39733baf6d375377c6eaf73d752493050baa7f, 17d801ad73f4e96527596a2fcb3fe74fc875bc94, a92362ce672ab7689fc33bbd036298697af630c4, 3750532cf1d4e2f7178fa086b60bd3d7c1a0aa7b, 080b3cc3b0eee18fce1745caa3167ae754efaa8e, 91a7769fc7ad4921ea3b1bb0fc9823b9e3bceb2f, 66f97df28cfe2db86da6b330ed5a8d125fe587be, 3fa869449f9c5ba49ff6af7d744ee56a8613c2a4, 4dbc937bb9d0a10edf2c2bd33c7df13157eea1e5, 79ab485722ff09ec29613b7e2b41a712f3cc3c3f, 4a81c2c0af0f7ba049ef44bff3f9f7b3702dada2, fced3a72e1e165c932ed192946f9c971d4f1d5f7, f3a176bfa5bb2e270ba5da839549bcaed0d12b87, cff41f88df68076b0d1497e96d3e324549cfc845) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

/easycla

moleske avatar Feb 05 '24 18:02 moleske

integration tests are failing cause they are using ginkgo v1, as that's what is currently in the main branch. this branch/pr updates the integration tests to use ginkgo v2. which will cause problems with v8/v7 branches when this is merged. This may be acceptable if we immediately work on porting those branches after this pr is merged

this pr won't go green until the main branch is updated to use ginkgo v2 in the integration tests

moleske avatar Feb 05 '24 22:02 moleske

Delivered via #2810

a-b avatar Mar 22 '24 22:03 a-b