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

Feature/upgrade testpack to ts4 #1189

Merged
merged 11 commits into from Sep 20, 2020
Merged

Conversation

johnnyreilly
Copy link
Member

@johnnyreilly johnnyreilly commented Sep 19, 2020

Hey @andrewbranch

I wonder if I could beg a little help? I've been upgrading ts-loaders comparison test pack to TypeScript 4.0. (Not my favourite job but there you go)

Anyway, a mystery has presented itself. The following comparison tests started erroring post upgrade, with no output coming from the transpileOnly tests.

Upon closer inspection, these tests have a common trait; they appear to contain no source code (by which I mean TypeScript / JavaScript and related code in the root of each tests directory):

I'm somewhat baffled as to what these tests actually test if there's no source code in the folder of each test. They're doing something as output is produced; I'm just a little stumped as to what! Forgive me for not knowing this myself, I have a feeling I did once but it escapes me now!

If you could set me straight I'd appreciate it - I'm pondering whether the testpack should feature these tests at all. They're all Project References tests as it happens.

I'm kind of puzzled as to whether these tests actually do anything! All help appreciated ❤️

cc @sheetalkamat - I think you may both have worked on these tests at some point, so I defer to your expertise!

@johnnyreilly
Copy link
Member Author

johnnyreilly commented Sep 20, 2020

I was debugging this, this morning and discovered that the --save-output mechanism was now not always saving output; it was doing it conditionally. And it so happened to show up with transpileOnly tests. I believe this is not what we want and so I reverted it with this commit:

64689d2#diff-9f602bf82ae3c45651a29c3751234e1bR287

// I believe we always want to copy this
// if (actual !== expected) {
fs.copySync(path.join(paths.actualOutput, file), path.join(paths.originalExpectedOutput, file));
// }

I subsequently regenerated the output for the tests that failed - let's see what happens.

Even if the tests now pass it still leaves the mystery (to me at least) of what these tests do!

@johnnyreilly johnnyreilly marked this pull request as ready for review September 20, 2020 13:34
@johnnyreilly johnnyreilly merged commit db5ea55 into master Sep 20, 2020
@johnnyreilly johnnyreilly deleted the feature/upgrade-testpack-to-ts4 branch September 20, 2020 18:15
@johnnyreilly johnnyreilly restored the feature/upgrade-testpack-to-ts4 branch September 20, 2020 18:15
@andrewbranch
Copy link
Contributor

Hummm, none of these tests look familiar to me, and I have no idea how/why they would have expected output but no test content. I think @sheetalkamat must have created these—I’m stumped.

@sheetalkamat
Copy link
Contributor

The test case was same as non composite or non watch api test case name so it just copied contents from that test case before starting the test.

@johnnyreilly
Copy link
Member Author

johnnyreilly commented Sep 21, 2020

Ahhhhhhhhh! - where can I spot the code that does the copying? I have had a scan through https://github.com/TypeStrong/ts-loader/blob/master/test/comparison-tests/create-and-execute-test.js but not spotted anything that looks like it would do that..

@johnnyreilly
Copy link
Member Author

Oh wait - I bet it's this

mkdirp.sync(paths.testStagingPath);

@sheetalkamat
Copy link
Contributor

https://github.com/TypeStrong/ts-loader/blob/master/test/comparison-tests/create-and-execute-test.js#L88

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.

None yet

3 participants