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

Manual Recurring Maintenance Tasks #1747

Open
rajsite opened this issue Jan 11, 2024 · 2 comments
Open

Manual Recurring Maintenance Tasks #1747

rajsite opened this issue Jan 11, 2024 · 2 comments
Assignees

Comments

@rajsite
Copy link
Member

rajsite commented Jan 11, 2024

🧹 Tech Debt

There are certain tasks that are difficult to automatically handle in source and cause regressions that may not be spotted.
This issue tracks those tasks, gives instructions for auditing them, and captures the last time they were audited.


Validate lifecycle overrides call super

Procedure

  • Verify that all overrides for connectedCallback and disconnectedCallback invoke super (easy to do by scanning two VSCode search results)
    image

Why is it not automated?

Last checked

  • 2024-09-05

Validate default label provider strings are aligned

Procedure

  • Check the nimble component label provider default translation strings (nimble-components/src/label-provider/**/label-token-defaults.ts) align with
    • Angular label provider directive default localization strings (nimble-angular/label-provider/**/ directives and tests)
    • Blazor wrappers and tests

Why not automated?

  • I think the localize tag helper used in the directives didn't make it easy to compose the strings from the constants exported from nimble components. Solutions were trying to do source code generation which seemed unnecessarily complex. They are just defaults, all meant to be overridden in specific app localizations anyway.

Last checked

  • 2024-09-05

Validate karma tests running on Safari

Procedure

  • Check that the nimble component tests running on Safari are passing.

Why not automated?

  • We run tests in WebKit as a proxy for Safari. The ROI of setting up a separate build agent just to run Safari on MacOS is low.

Last checked

  • 2024-09-05

Keep pinned dependencies up-to-date

Procedure

Why not automated?

  • Because for some reason the playwright version on nuget does not always align with the npm package. So if we let rennovate bump the playwright versions may not align across packages.
  • Because we want the JS and Nuget version of playwright to update at the same time (rennovate likely could be configured to do this)

Last checked

  • 2024-06-05

Review and update status of Blocked issues

Procedure

  • Review list of blocked issues, and if there has been a change, add a comment with the newest status and remove the Blocked label if appropriate.

Why not automated?

  • Each issue requires a manual review process.

Last checked

  • 2024-06-05

Review disabled tests

Procedure

  • Search for all disabled tests (xit, xdescribe) and ensure they have comments linking to issues.
  • If a test has been disabled for more than a few weeks, try to get it some attention.

Why not automated?

  • Each test requires a manual review process.

Last checked

  • Never

Audit dependency vulnerabilities

Procedure

  • Locally run npm audit or inspect the npm ci step of a recent pipeline build to see what vulnerabilities are reported
  • If there are vulnerabilities with fixes available, try to fix them using one of these approaches. Use your (or code owner) judgement to pick between them based on how well they eliminate vulnerabilities and what other issues they introduce.
    • In a local branch, delete node_modules and package-lock.json, run npm install, and submit the regenerated package-lock.json
    • Run npm audit fix and submit the regenerated package-lock.json.

Why not automated?

  • When we added npm audit to pipelines we found it annoying that pipelines immediately broke when new vulnerabilities were discovered. Alternatives like using security tooling haven't been prioritized.

Last checked

  • 2024-09-05
@rajsite rajsite added tech debt triage New issue that needs to be reviewed labels Jan 11, 2024
rajsite added a commit that referenced this issue Jan 16, 2024
…e clean-up (#1748)

# Pull Request

## 🤨 Rationale

Fixes a couple of small issues noticed recently:

- Tests were printing out console warnings
- Some component callback lifecycles were not calling their super
implementation
- When test failures happened the current spec would stop making it
difficult to see all the failures and handle them

## 👩‍💻 Implementation

- Elevated console warn and console errors to test failures
- Updated lifecycle callbacks to call super (and renamed some methods
that weren't actually lifecycle callbacks, which was my idea, I was
wrong 😅). Created a Manual Regression Validation issue to hopefully
track that a little better in the future:
#1747
- Updated the jasmine configuration to continue running a test suite
even on failure. We can see if it is useful and easily switch back if it
is annoying. Was useful locally.
- Filed an issue for some Wafer Map tests that are running incorrectly:
#1746

## 🧪 Testing

Rely on CI.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Jan 23, 2024
@rajsite rajsite pinned this issue Feb 2, 2024
@m-akinc m-akinc changed the title Manual Regression Validations Manual Recurring Maintenance Tasks Feb 20, 2024
@m-akinc m-akinc added the team review Flag issue for team discussion and review label Mar 1, 2024
@m-akinc m-akinc removed the team review Flag issue for team discussion and review label Mar 12, 2024
jattasNI added a commit that referenced this issue Mar 13, 2024
…omponents (#1927)

# Pull Request

## 🤨 Rationale

#1747 has a maintenance task to ensure the default strings in Angular
and Blazor directives match the default strings in nimble-components.

## 👩‍💻 Implementation

Some labels were only provided in nimble-components, not nimble-angular
or Blazor. Copied those values over following existing patterns in
directives and test files.

## 🧪 Testing

Updated tests but otherwise none since this is a mechanical change.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <rajsite@users.noreply.github.com>
rajsite added a commit that referenced this issue Mar 18, 2024
# Pull Request

## 🤨 Rationale

One of the tasks in #1747 is to run nimble-components tests in WebKit
and see if there are any new failures. There are several.

We also wanted to start tracking specific issues for each root cause
rather than catch all issues like #1074 and #1075.

## 👩‍💻 Implementation

Marked each failing test with one of these specific issues:
- #1936
- #1938 TODO: ALSO FILE AZDO BUG
- #1939 
- #1940 
- #1942 
- #1943

Also re-enabled a couple of table header tests in Firefox which are now
passing.

## 🧪 Testing

Ran tests in Playwright webkit browser, Firefox, and Safari and and all
the enabled tests now pass.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <rajsite@users.noreply.github.com>
@jattasNI jattasNI removed their assignment Mar 21, 2024
@jattasNI
Copy link
Contributor

Manual tasks complete for this quarter 🎉

rajsite pushed a commit that referenced this issue Mar 22, 2024
# Pull Request

## 🤨 Rationale

#1747 has a recurring maintenance task to update the version of
Playwright to the latest version that's available in npm and Nuget. We
were on 1.40.0, the [latest on npm is
1.42.1](https://www.npmjs.com/package/playwright?activeTab=versions),
and the [latest on Nuget is
1.42.0](https://www.nuget.org/packages/Microsoft.Playwright)

[Detailed release
notes](https://github.com/microsoft/playwright/releases).

## 👩‍💻 Implementation

1. Searched for "1.40.0" in package.json files and updated to "1.42.0".
2. Ran `npm install` to regenerate package-lock.json

## 🧪 Testing

1. Verified locally that
`packages/nimble-blazor/build/generate-playwright-version-properties/dist/Playwright.PackageVersion.props`
is regenerated and the build and test commands pass.
2. Otherwise relying on PR build

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@m-akinc m-akinc added triage New issue that needs to be reviewed and removed triage New issue that needs to be reviewed labels Jun 3, 2024
@m-akinc m-akinc assigned m-akinc and unassigned m-akinc Jun 4, 2024
jattasNI added a commit that referenced this issue Jul 9, 2024
# Pull Request

## 🤨 Rationale

After #2234 fixed #1732, `npm audit` still returned one critical
vulnerability in `minimist`.

## 👩‍💻 Implementation

I ran `npm audit fix` as the `audit` command suggested. Unlike previous
times we tried this, the diff was very minimal and didn't seem very
disruptive so I think it's worth submitting. Thanks to Renovate for
keeping our package-lock.json fairly up to date!

I considered re-adding the `npm audit` command in `main.yml` but I've
never loved the way that can arbitrarily break our pipeline. Instead I
propose relying on monthly Renovate updates and adding a quarterly task
to #1747 to run `audit` and fix any low-hanging vulnerabilities. Open to
feedback on this!

## 🧪 Testing

Running npm audit now prints "found 0 vulnerabilities".

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@jattasNI
Copy link
Contributor

jattasNI commented Jul 9, 2024

I added a step to manually run npm audit as described in #2240.

@m-akinc m-akinc added the triage New issue that needs to be reviewed label Aug 28, 2024
@jattasNI jattasNI removed the triage New issue that needs to be reviewed label Sep 3, 2024
@jattasNI jattasNI self-assigned this Sep 5, 2024
rajsite pushed a commit that referenced this issue Sep 5, 2024
# Pull Request

## 🤨 Rationale

#1747 has a task to validate that label provider strings are synced
across all frameworks. There was a new "Loading..." string that was
missing for Angular and Blazor.

## 👩‍💻 Implementation

Scanned all the label provider code and tests across all frameworks and
ensured parity.

## 🧪 Testing

PR build

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

No branches or pull requests

3 participants