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

🏗 ♻️ Fix type errors in /build-system/tasks/e2e #33166

Merged
merged 4 commits into from
Apr 14, 2021

Conversation

rileyajones
Copy link
Contributor

/build-system/tasks/e2e is directory which contains the largest number of type errors of any of build-system tasks. This PR is intended to be non functional and should resolve all simple type errors within the subdirectory. After this lands there will still be a handful of non trivial errors requiring functional changes that will be fixed as part of another PR.

#28387

@rileyajones rileyajones force-pushed the tsc-e2e branch 2 times, most recently from 4e788a0 to b2d0ac1 Compare March 9, 2021 22:15
@rileyajones rileyajones marked this pull request as ready for review March 10, 2021 17:43
@amp-owners-bot
Copy link

amp-owners-bot bot commented Mar 10, 2021

Hey @estherkim! These files were changed:

build-system/tasks/e2e/amp-driver.js
build-system/tasks/e2e/controller-promise.js
build-system/tasks/e2e/describes-e2e.js
build-system/tasks/e2e/expect.js
build-system/tasks/e2e/functional-test-controller.js
build-system/tasks/e2e/index.js
build-system/tasks/e2e/mocha-custom-json-reporter.js
build-system/tasks/e2e/network-logger.js
build-system/tasks/e2e/puppeteer-controller.js
build-system/tasks/e2e/selenium-webdriver-controller.js

build-system/tasks/e2e/selenium-webdriver-controller.js Outdated Show resolved Hide resolved
build-system/tasks/e2e/selenium-webdriver-controller.js Outdated Show resolved Hide resolved
build-system/tsconfig.json Outdated Show resolved Hide resolved
Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

Getting closer to zero errors! LGTM with a few more comments. Okay to merge after they are addressed and CI is green.

build-system/tasks/e2e/functional-test-controller.js Outdated Show resolved Hide resolved
Comment on lines 29 to 32
Browser, // eslint-disable-line no-unused-vars
JSHandle, // eslint-disable-line no-unused-vars
Page, // eslint-disable-line no-unused-vars
ElementHandle: PuppeteerHandle, // eslint-disable-line no-unused-vars
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to avoid these?

Copy link
Contributor Author

@rileyajones rileyajones Apr 13, 2021

Choose a reason for hiding this comment

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

I could condense them, or else I could try to do some dynamic typing or just drop type enforcement. I'll update this PR soon with all these condensed to just puppeteer.

build-system/tasks/e2e/selenium-webdriver-controller.js Outdated Show resolved Hide resolved
build-system/tsconfig.json Outdated Show resolved Hide resolved
Comment on lines +82 to 96
* @param {function(CONDITION, function(VALUE): ?DERIVED): Promise<RES>} wait
* @param {function(VALUE): MUTANT} mutate
* @return {function(CONDITION, function(MUTANT): ?DERIVED): Promise<RES>}
* @template CONDITION
* @template MUTANT
* @template DERIVED
* @template VALUE
* @template RES
*/
function wrapWait(wait, mutate) {
return (condition, opt_mutate) => {
opt_mutate = opt_mutate || ((x) => x);
opt_mutate = opt_mutate || ((x) => /** @type {*} */ (x));
return wait(condition, (value) => opt_mutate(mutate(value)));
};
}
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 was a heck of a puzzle. More than open to changing type names etc...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Go for it!

@rileyajones rileyajones merged commit 5ebf84e into ampproject:main Apr 14, 2021
rileyajones added a commit to rileyajones/amphtml that referenced this pull request Apr 16, 2021
* fix errors in /tasts/e2e

* update typing in puppeteer-controller

* solve controller-promise typing puzzle
rochapablo pushed a commit to rochapablo/amphtml that referenced this pull request Aug 30, 2021
* fix errors in /tasts/e2e

* update typing in puppeteer-controller

* solve controller-promise typing puzzle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants