Skip to content

Conversation

@pushkine
Copy link
Contributor

@pushkine pushkine commented Jan 4, 2021

Extracted htmlx2jsx and svelte2tsx tests common code into a test_sample function in helpers.js
Added check for random unused files in samples (warns locally, throws in CI)
Added auto generation of the expected output if absent (marks test as skipped and logs a notice at the end locally, throws in CI)
Made samples starting with a dot show up as skipped tests instead of silent

Made the htmlxparser timeout test generate its input dynamically

Removed checks for .solo folders in 3 places for a single CI-only match-all glob in index

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

These are good changes! I left some minor refactoring comments in there. Also it seems that you used tabs instead of spaces throughout. I don't know if you use prettier, but if you do, it should pick up the configuration if you open the workspace from the root.

for (const name of unchecked)
if (name === fileName || (name.startsWith("*") && fileName.endsWith(name.slice(1)))) {
unchecked.delete(name);
continue a;
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove that continue statement and make the other if an else if instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I restructured it and corrected some things but I don't think you can get much better due to the wildcard thing

@pushkine
Copy link
Contributor Author

pushkine commented Jan 7, 2021

it should pick up the configuration if you open the workspace from the root

Ah, svelte2tsx/test was in .prettierignore, I removed it if that's okay

@dummdidumm
Copy link
Member

it should pick up the configuration if you open the workspace from the root

Ah, svelte2tsx/test was in .prettierignore, I removed it if that's okay

Could you alter it so that the tsx/jsx files are ignored? Otherwise it might be very cumbersome to change these files.

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Thanks!

@dummdidumm dummdidumm merged commit c3de841 into sveltejs:master Jan 8, 2021
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.

2 participants