Skip to content

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.

4 participants