alsatian icon indicating copy to clipboard operation
alsatian copied to clipboard

Support for tests written with baseUrl in mind

Open paarthenon opened this issue 7 years ago • 20 comments

Do you fine folks have a good way of handling test cases written with the baseUrl property in mind? For example I'd like to be able to do something like

 import * as promiseUtils from 'src/utils/promise'

in my test file rather than using unwieldy relative paths. However, this does not resolve in the alsatian test runner.

alsatian build/test/**/*.spec.js

returns

(node:22012) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Cannot find module 'src/utils/promise'

I see why it fails, but I would like to be able to do this.

paarthenon avatar Apr 24 '17 03:04 paarthenon

Hey @paarthenon,

This certainly looks like something we'd want to support. Could you send us a link to the repository so we can investigate resolving this for you please? :)

jamesadarich avatar Apr 24 '17 06:04 jamesadarich

@jamesrichford 👍 dogs are the best profile pics

My real use case is paarthenon/pixiv-assistant but this is something I actively work on so the structure may change. I created a simplified example showing this in action at:

https://github.com/paarth-minimal-example/alsatian-issue-367

you should be able to replicate with

npm install
npm run build
npm run test

paarthenon avatar Apr 24 '17 13:04 paarthenon

Fyi @paarthenon you can simply use tsc and alsatian in your package.json scripts, because they are installed as dev dependencies.

Jameskmonger avatar Apr 24 '17 13:04 Jameskmonger

huh, so I can. For future readers you can find details here:

https://docs.npmjs.com/cli/run-script

Thanks James! The benefits of social coding I guess.

paarthenon avatar Apr 24 '17 14:04 paarthenon

Thanks @paarthenon for the awesome simple example and your opinion on dog based avatars, I very much agree :)

I've had a little investigate into this and seems like we'll need to add some custom support here probably worth including paths as well as baseUrl.

I propose something along the lines of setting the tsconfig.json to import into alsatian e.g.

alsatian **/*.spec.js --tsconfig ./location/of/tsconfig.json

Thoughts @paarthenon and @Jameskmonger?

jamesadarich avatar Apr 24 '17 18:04 jamesadarich

Inspiration can be found at https://www.npmjs.com/package/tsconfig-paths

jamesadarich avatar Apr 24 '17 18:04 jamesadarich

That structure seems sane to me @jamesrichford.

I'm guessing module resolution would conceptually be something like path.resolve(outDir, baseUrl, requirePath)?

The bundlers I've used (systemjs/webpack) don't seem to have run into issues. I wonder how they manage it.

paarthenon avatar Apr 24 '17 18:04 paarthenon

Come to think of it, are there going to be other consumers of alsatian that have this import style like babel or systemjs users? Would there be benefit in something more generic to specify than the tsconfig file so they could benefit?

That said I have no idea if these people or issues even exist (or if we care about them even if they do). I figured I'd bring it up while we're discussing it.

paarthenon avatar Apr 24 '17 18:04 paarthenon

@jamesrichford I agree with @paarthenon in that we should not force tsconfig - the require/import aliases can come from a variety of sources.

We also need to consider the fact that Webpack (and likely others) allow you to have multiple aliases:

Config

alias: {
  Utilities: path.resolve(__dirname, 'src/utilities/'),
  Templates: path.resolve(__dirname, 'src/templates/')
}

Usage

import { SomeUtility } from "Utilities/some-utility";
import { ReallyCoolTemplate, BoringTemplate } from "Templates/index";

How will we manage that?

I think that this needs more consideration.

Jameskmonger avatar Apr 24 '17 22:04 Jameskmonger

@Jameskmonger I was thinking about some sort of require wrapper. It looks like the tsconfig-paths project @jamesrichford linked earlier isn't strictly dependent on a tsconfig, you could pass in your own paths object. Maybe that could be leveraged directly? It feels like rolling your own solution might be out of scope of alsatian.

I don't know if it would work but it would be nice if that could be leveraged directly with an endpoint available on alsatian to pass in a custom paths mapping (+ a simplified system where you pass in the tsconfig location)

paarthenon avatar Apr 24 '17 23:04 paarthenon

How do other test frameworks handle this? It's not a TypeScript specific issue so I imagine Jasmine etc will have something for it.

Jameskmonger avatar Apr 25 '17 08:04 Jameskmonger

I think that module loaders like webpack etc should be exempt because it's the job to resolve paths anyway so we don't need to worry about them. Just for those that are utilising references that won't be resolved by node normally e.g. typescript paths.

Next step is comparing Traceur, Typescript, Babel to see how they set this up and then devise a plan.

As for other test frameworks @Jameskmonger they don't handle this case this is why the module I linked to exists :)

jamesadarich avatar Apr 29 '17 09:04 jamesadarich

@jamesrichford @Jameskmonger I've implemented a fix using the tsconfig-paths library you listed earlier. I've created a new branch of the example project called fix that features the changes. You can view the diff here:

https://github.com/paarth-minimal-example/alsatian-issue-367/compare/fix1

I'm glad this is working but as you can see the test script in the package.json is less than ideal. Any suggestions on cleaning that up?

paarthenon avatar May 22 '17 05:05 paarthenon

Any progress on this? I've just ran into this one with aliases

hisuwh avatar Sep 24 '20 17:09 hisuwh

For context as OP I posted that fix comment and never really heard back, I imagine they were busy with other things. It looks like there were some tags applied to this issue recently, but I'm no longer in this loop. I ended up switching to inline tests with jest. @jamesadarich was there more that came of it?

paarthenon avatar Sep 24 '20 19:09 paarthenon

@hisuwh @paarthenon I solved this in creature-chess with this file (lines 8-10)

https://github.com/Jameskmonger/creature-chess/blob/master/run-tests.ts

It's a little ugly though, as I said, to force tsconfig here.

I think a better solution would be to remove that, and do this:

ts-node -r tsconfig-paths/register ./run-tests.ts

Jameskmonger avatar Sep 24 '20 20:09 Jameskmonger

I've been working on this for the next major release. It's already available in the vscode extension I believe if you wanna try it out. It requires the usage of the new .alsatianrc.json file.

If anyone wants to earn those jacket hacktoberfest points let me know and can give out some small pieces to be done for this and similar stuff :)

jamesadarich avatar Sep 24 '20 20:09 jamesadarich

Is the .alsatianrc.json file going to be used by the main alsatian cli as well as the extension in the new version?

hisuwh avatar Sep 25 '20 10:09 hisuwh

@jamesadarich

If anyone wants to earn those jacket hacktoberfest points let me know and can give out some small pieces to be done for this and similar stuff :)

I'm interested

hisuwh avatar Sep 25 '20 10:09 hisuwh

@hisuwh yes that's correct trying to unify setup and make config easier

jamesadarich avatar Sep 27 '20 22:09 jamesadarich