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

Move openTerminal() to common util file #2711

Merged
merged 4 commits into from
Feb 13, 2020
Merged

Conversation

jmbockhorst
Copy link
Contributor

Part of #2699. Moved openTerminal() into a common util file that is used by everything in test/api/. I'm not sure how to make this accessible to the addon test files.

@jmbockhorst
Copy link
Contributor Author

I found a way to make the addons use the shared functions, but I don't love it because they have to import from ../../../out-test/api/TestUtils. I found no way to make relative paths work because mocha can't resolve them in the js files.

A better approach seems to be to use something like ts-node with mocha and test the ts files directly like this: mocha -r ts-node/register **/*.api.ts. However this caused more problems (I think due to the complex structure of the project), so I opted for the current solution.

@Tyriar Tyriar added this to the 4.5.0 milestone Feb 12, 2020
"strict": true,
"types": [
"../../../node_modules/@types/mocha",
"../../../out-test/api/TestUtils"
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, not really a far of referencing out but it does indeed seem to work and what I preferred doesn't (because mocha doesn't accept it as you mention). The main problem I see with it now is that the files that reference it have errors before running the first build. I guess we can go with this, if it ends up causing problems we can always go back to duplicating for addons 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I wasn't a fan either, but I think it's better than duplicated code.

test/api/tsconfig.json Outdated Show resolved Hide resolved
@Tyriar Tyriar merged commit c5593bd into xtermjs:master Feb 13, 2020
@Tyriar Tyriar linked an issue Feb 16, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pull common API test helper functions into common file
2 participants