nebari upgrade CLI tests

[sblair-metrostar]

Reference Issues or PRs

#1862

Fixes #1961
Fixes #1962

What does this implement/fix?

Put a x in the boxes that apply

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds a feature)
• Breaking change (fix or feature that would cause existing features not to work as expected)
• Documentation Update
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build related changes
• Other (please describe): Unit tests

Testing

• Did you test the pull request locally?
• Did you add new tests?

Any other comments?

[iameskild]

Thanks @sblair-metrostar for these tests!

I currently see four tests failing. One is a test_upgrade_does_nothing_on_same_version and the other three appear to be tests written for the 0.4.0 upgrade. For the old 0.4.0 upgrade tests, I would be alright if we tore those out given how old they are and I don't think the maintenance burden is worth it.

[sblair-metrostar]

Thanks @sblair-metrostar for these tests!

I currently see four tests failing. One is a test_upgrade_does_nothing_on_same_version and the other three appear to be tests written for the 0.4.0 upgrade. For the old 0.4.0 upgrade tests, I would be alright if we tore those out given how old they are and I don't think the maintenance burden is worth it.

No they're failing as a side effect of my attempt to fix the dask_worker section upgrade. It looks like there is a bug somewhere but it's not what I thought it was. Will reach out this morning to run it by you.

[sblair-metrostar]

@iameskild I've updated the details of #1961. There was a bug with that behavior but wasn't what I thought it was initially.

[iameskild]

Thanks for the fixes @sblair-metrostar!

One question I forgot to ask earlier was around new releases. Before cut a new release, we need add a new Upgrade_ class and, with these changes, we should also add another test for that upgrade. Is that how you were also envisioning this?

[sblair-metrostar]

Thanks for the fixes @sblair-metrostar!

One question I forgot to ask earlier was around new releases. Before cut a new release, we need add a new Upgrade_ class and, with these changes, we should also add another test for that upgrade. Is that how you were also envisioning this?

That would be my expectation, yes. There's some quirkiness around whether or not there's a release specific UpgradeStep in nebari or not and a few tests that only work with the "current" version we'll want to keep an eye on. But we should just be able to add a test that covers the 7.2 to 9.1 upgrade path with any specific yaml assertions we'd expect in addition to the version getting ticked up.

[iameskild]

Related to #1955