Skip to content

Handle nonmaster branches in user suite tests #48128

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

Merged
merged 5 commits into from
Mar 9, 2022

Conversation

weswigham
Copy link
Member

These are just the two user tests that were failing because they couldn't fetch anything (meaning the main/master swap was very
ungraceful and left no redirects behind) - in a similar vein, other user or docker tests may be silently pulling age-old code from an unused master branch. A comprehensive audit of the test repos is needed. 🙁

@weswigham weswigham requested a review from sandersn March 5, 2022 02:16
@typescript-bot typescript-bot assigned weswigham and unassigned weswigham Mar 5, 2022
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 5, 2022
@weswigham
Copy link
Member Author

@sandersn I've updated this to fix the branches for all test.json files. I don't know if the inline task needs to be updated any to handle the differing branch names. In addition, I also fixed the builds for the two docker suite tests with broken builds. (vue-next moves to vuejs/core and now uses pnpm instead of yarn, vscode uses a newer node version now).

@weswigham
Copy link
Member Author

c5f7b7c snapshots current user suite baseline results with these fixes (docker suite's a bigger run, so that'll happen separately). I've already opened #48163 from my first glance at the logs, however there are some more issues I'll flag for further investigation here.

Standard output:
node_modules/mongoose/index.d.ts(438,37): error TS2589: Type instantiation is excessively deep and possibly infinite.
node_modules/mongoose/index.d.ts(1194,5): error TS2589: Type instantiation is excessively deep and possibly infinite.
node_modules/mongoose/index.d.ts(1195,5): error TS2589: Type instantiation is excessively deep and possibly infinite.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm pretty sure we don't want mongoose having an error like this - I imagine we need to look into this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Error is on a Query<any, this> inside the Document class, where Query<ResultType, DocType extends Document>. This is a really simple type and absolutely shouldn't infini-stack. The other two are on Schema instantiations (which is also just a class with a type argument); I think the commonality is the FilterQuery type usage within both; need to investigate this one more; making a minimal repro will take a bit.

@@ -7,6 +7,7 @@ node_modules/clone/clone.js(176,14): error TS2532: Object is possibly 'undefined
node_modules/clone/clone.js(186,16): error TS2403: Subsequent variable declarations must have the same type. Variable 'i' must be of type 'string', but here has type 'number'.
node_modules/clone/clone.js(186,23): error TS2365: Operator '<' cannot be applied to types 'string' and 'number'.
node_modules/clone/clone.js(186,52): error TS2356: An arithmetic operand must be of type 'any', 'number', 'bigint' or an enum type.
node_modules/clone/clone.js(255,35): error TS2774: This condition will always return true since this function is always defined. Did you mean to call it instead?
Copy link
Member Author

Choose a reason for hiding this comment

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

This error is totally wrong - the pattern is

if (typeof module === 'object' && module.exports) {
  module.exports = clone;
}
  • the inner module.exports assignment being treated as unconditional is making us error on the check to perform the assignment.
    Minimally, we need to add an exception to the error where module.exports checks, specifically, aren't considered always-defined (since they're runtime module system checks).

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,20 @@
Exit Code: 2
Standard output:
lib/dependencies/WorkerPlugin.js(268,23): error TS2339: Property 'name' does not exist on type 'Record<string, any> | {}'.
Copy link
Member Author

Choose a reason for hiding this comment

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

This variable flows in from a location pretty explicitly typed as Record<string, any> - the | {} seems pretty bogus; not sure how it got in there, but it definitely seems like a bug.

@sandersn
Copy link
Member

sandersn commented Mar 8, 2022

The inline user tests do indeed need to be updated. I think after we finish auditing the existing tests we should shut down the infrastructure and just keep those up-to-date.

I'll update the inline user tests in the next few days.

@weswigham
Copy link
Member Author

I think after we finish auditing the existing tests we should shut down the infrastructure and just keep those up-to-date.

Problem is the inline user tests only capture PR variations - they never capture when main breaks something or a project introduces a change that breaks the test itself. So it's useful for PRs, but useless for testing main. The suites share a test definition, but have differing goals and purposes.

@weswigham weswigham merged commit c70be8b into microsoft:main Mar 9, 2022
sandersn added a commit to microsoft/typescript-error-deltas that referenced this pull request Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants