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

tech-story: [M3-8940] - Dev Tools fixes and improvements #11328

Merged
merged 12 commits into from
Dec 6, 2024

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Nov 26, 2024

Description 📝

Long overdue updates to our beloved Dev Tools. This PR does not touch any client code. Brings a lot of improvements and fixes to presets and mocking.

Note

Code quality/readability/reusability is not as high as it would be for client code, tho I tried best to achieve that while working quickly. The goal aims to help the developer experience first 💨

Changes 🔄

  • Allow JSON/nested feature flags toggling (recursively)
  • Fix DevTools panel overflow
  • Improve re-haul Account/Profile mocking
  • Style improvements

Target release date 🗓️

Please specify a release date (and environment, if applicable) to guarantee timely review of this PR. If exact date is not known, please approximate and update it as needed.

Preview 📷

Screenshot 2024-11-26 at 14 38 03

Screenshot 2024-11-26 at 14 38 18

How to test 🧪

Verification steps

  • Play with MSW, verify
    • Nested feature flags
    • Profile and accounts
    • No regressions in saving, storing mock data etc

As an Author, I have considered 🤔

  • 👀 Doing a self review

  • ❔ Our contribution guidelines

  • 🤏 Splitting feature into small PRs

  • ➕ Adding a changeset

  • 🧪 Providing/improving test coverage

  • 🔐 Removing all sensitive information from the code and PR description

  • 🚩 Using a feature flag to protect the release

  • 👣 Providing comprehensive reproduction steps

  • 📑 Providing or updating our documentation

  • 🕛 Scheduling a pair reviewing session

  • 📱 Providing mobile support

  • ♿ Providing accessibility support

  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@abailly-akamai abailly-akamai self-assigned this Nov 26, 2024
@abailly-akamai abailly-akamai changed the title tech-story: [M3-8940] - MSW fixes and improvements tech-story: [M3-8940] - Dev Tools fixes and improvements Nov 26, 2024
@abailly-akamai abailly-akamai marked this pull request as ready for review November 26, 2024 20:03
@abailly-akamai abailly-akamai requested a review from a team as a code owner November 26, 2024 20:03
@abailly-akamai abailly-akamai requested review from coliu-akamai, pmakode-akamai and jaalah-akamai and removed request for a team November 26, 2024 20:03
'VPCs',
] as const;

export type AccountCapability = typeof accountCapabilities[number];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This changes nothing to the types, just allows to export the values

</details>
</ul>
);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allowing recursiveness so the JSON flag can be as deep as needed and represented here

Left to do: handle the cases for when a flag is something else than a boolean, which we rarely do


const FieldWrapper = ({ children }: { children: React.ReactNode }) => {
return <div className="dev-tools__modal-form__field">{children}</div>;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before we were only providing a static factory override. This updates allows much more flexibility for both the response.

@abailly-akamai abailly-akamai force-pushed the M3-8940 branch 2 times, most recently from 674f537 to 86d2c60 Compare November 27, 2024 13:41
Copy link

github-actions bot commented Nov 27, 2024

Coverage Report:
Base Coverage: 86.85%
Current Coverage: 86.85%

Copy link
Contributor

@jaalah-akamai jaalah-akamai left a comment

Choose a reason for hiding this comment

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

Nice work overall, I actually used the account preset for my testing today for OBJ. 🔥

Copy link
Contributor

@jdamore-linode jdamore-linode left a comment

Choose a reason for hiding this comment

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

This is extremely cool @abailly-akamai, thanks for taking the time to add all this!

One technical question -- I was just messing around with the new feature flag work and tried to add a new JSON flag, but it's showing up in the dev tools as a checkbox. Can you spot what I did wrong?

// featureFlags.ts
interface JustMessingAroundFlag extends BaseFeatureFlag {
  optionA: boolean;
  optionB: boolean;
}

export interface Flags {
  justMessingAround: JustMessingAroundFlag;
  // ...
}
// FeatureFlagTool.tsx
const options: { flag: keyof Flags; label: string }[] = [
  { flag: 'justMessingAround', label: 'Just Messing Around' },
  // ...
];

Screenshot 2024-12-04 at 11 47 57 AM

Existing flags seem to work fine -- for example, I added the cloudManagerDesignUpdatesBanner flag to the array in FeatureFlagTool.tsx, and it showed up as expected, so I'm thinking I must be doing something wrong with the way I've set up my flag.

Either way, these are all such nice/powerful improvements! Thanks again Alban!

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 1 failing test on test run #8 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
1 Failing464 Passing2 Skipped100m 41s

Details

Failing Tests
SpecTest
smoke-billing-activity.spec.tsBilling Activity Feed » displays correct timezone for invoice and payment dates

Troubleshooting

Use this command to re-run the failing tests:

yarn cy:run -s "cypress/e2e/core/billing/smoke-billing-activity.spec.ts"

@abailly-akamai
Copy link
Contributor Author

@jdamore-linode could it simply be the fact that justMessingAround is typed but does not return any actual value from LD?

@abailly-akamai abailly-akamai merged commit 5ac188d into linode:develop Dec 6, 2024
22 of 23 checks passed
@jdamore-linode
Copy link
Contributor

@jdamore-linode could it simply be the fact that justMessingAround is typed but does not return any actual value from LD?

Ah thanks! I didn't realize they had to be tied to the actual Launch Darkly data since it's being mocked.

Copy link

cypress bot commented Dec 6, 2024

Cloud Manager E2E    Run #6932

Run Properties:  status check failed Failed #6932  •  git commit 5ac188d110: tech-story: [M3-8940] - Dev Tools fixes and improvements (#11328)
Project Cloud Manager E2E
Branch Review develop
Run status status check failed Failed #6932
Run duration 33m 53s
Commit git commit 5ac188d110: tech-story: [M3-8940] - Dev Tools fixes and improvements (#11328)
Committer Alban Bailly
View all properties for this run ↗︎

Test results
Tests that failed  Failures 2
Tests that were flaky  Flaky 5
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 464
View all changes introduced in this branch ↗︎

Tests for review

Failed  linodes/clone-linode.spec.ts • 1 failed test

View Output Video

Test Artifacts
clone linode > can clone a Linode from Linode details page Screenshots Video
Failed  billing/smoke-billing-activity.spec.ts • 1 failed test

View Output Video

Test Artifacts
Billing Activity Feed > displays correct timezone for invoice and payment dates Screenshots Video
Flakiness  linodes/switch-linode-state.spec.ts • 1 flaky test

View Output Video

Test Artifacts
switch linode state > reboots a linode from landing page Screenshots Video
Flakiness  linodes/resize-linode.spec.ts • 1 flaky test

View Output Video

Test Artifacts
resize linode > resizes a linode by increasing size when offline: cold migration Screenshots Video
Flakiness  linodes/linode-config.spec.ts • 1 flaky test

View Output Video

Test Artifacts
Linode Config management > End-to-End > Clones a config Screenshots Video
Flakiness  kubernetes/lke-create.spec.ts • 1 flaky test

View Output Video

Test Artifacts
LKE Cluster Creation > can create an LKE cluster Screenshots Video
Flakiness  linodes/create-linode-with-user-data.spec.ts • 1 flaky test

View Output Video

Test Artifacts
Create Linode with user data > cannot specify user data when selected image does not support it Screenshots Video

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Add'tl Approval Needed Waiting on another approval! Ready for Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants