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

typescript tests #3273

Closed
kedashoe opened this issue Apr 23, 2022 · 19 comments
Closed

typescript tests #3273

kedashoe opened this issue Apr 23, 2022 · 19 comments

Comments

@kedashoe
Copy link
Contributor

kedashoe commented Apr 23, 2022

.d.ts files in ./types needing test coverage:

  • __
  • add
  • addIndex
  • adjust
  • all
  • allPass
  • always
  • and
  • andThen
  • any
  • anyPass
  • ap
  • aperture
  • append
  • apply
  • applySpec
  • applyTo
  • ascend
  • assoc
  • assocPath
  • binary
  • bind
  • both
  • call
  • chain
  • clamp
  • clone
  • collectBy
  • comparator
  • complement
  • compose
  • composeWith
  • concat
  • cond
  • construct
  • constructN
  • converge
  • count
  • countBy
  • curry
  • curryN
  • dec
  • defaultTo
  • descend
  • difference
  • differenceWith
  • dissoc
  • dissocPath
  • divide
  • drop
  • dropLast
  • dropLastWhile
  • dropRepeats
  • dropRepeatsBy
  • dropRepeatsWith
  • dropWhile
  • either
  • empty
  • endsWith
  • eqBy
  • eqProps
  • equals
  • evolve
  • F
  • filter
  • find
  • findIndex
  • findLast
  • findLastIndex
  • flatten
  • flip
  • forEach
  • forEachObjIndexed
  • fromPairs
  • groupBy
  • groupWith
  • gt
  • gte
  • has
  • hasIn
  • hasPath
  • head
  • identical
  • identity
  • ifElse
  • inc
  • includes
  • indexBy
  • indexOf
  • init
  • innerJoin
  • insert
  • insertAll
  • intersection
  • intersperse
  • into
  • invert
  • invertObj
  • invoker
  • is
  • isEmpty
  • isNil
  • isNotNil
  • join
  • juxt
  • keys
  • keysIn
  • last
  • lastIndexOf
  • length
  • lens
  • lensIndex
  • lensPath
  • lensProp
  • lift
  • liftN
  • lt
  • lte
  • map
  • mapAccum
  • mapAccumRight
  • mapObjIndexed
  • match
  • mathMod
  • max
  • maxBy
  • mean
  • median
  • memoizeWith
  • mergeAll
  • mergeDeepLeft
  • mergeDeepRight
  • mergeDeepWith
  • mergeDeepWithKey
  • mergeLeft
  • mergeRight
  • mergeWith
  • mergeWithKey
  • min
  • minBy
  • modulo
  • move
  • multiply
  • nAry
  • negate
  • none
  • not
  • nth
  • nthArg
  • o
  • objOf
  • of
  • omit
  • on
  • once
  • or
  • otherwise
  • over
  • pair
  • partial
  • partialObject
  • partialRight
  • partition
  • path
  • pathEq
  • pathOr
  • paths
  • pathSatisfies
  • pick
  • pickAll
  • pickBy
  • pipe
  • pipeWith
  • pluck
  • prepend
  • product
  • project
  • promap
  • prop
  • propEq
  • propIs
  • propOr
  • props
  • propSatisfies
  • range
  • reduce
  • reduceBy
  • reduced
  • reduceRight
  • reduceWhile
  • reject
  • remove
  • repeat
  • replace
  • reverse
  • scan
  • set
  • slice
  • sort
  • sortBy
  • sortWith
  • split
  • splitAt
  • splitEvery
  • splitWhen
  • splitWhenever
  • startsWith
  • subtract
  • sum
  • symmetricDifference
  • symmetricDifferenceWith
  • T
  • tail
  • take
  • takeLast
  • takeLastWhile
  • takeWhile
  • tap
  • test
  • thunkify
  • times
  • toLower
  • toPairs
  • toPairsIn
  • toString
  • toUpper
  • transduce
  • transpose
  • traverse
  • trim
  • tryCatch
  • type
  • unapply
  • unary
  • uncurryN
  • unfold
  • union
  • unionWith
  • uniq
  • uniqBy
  • uniqWith
  • unless
  • unnest
  • until
  • update
  • useWith
  • values
  • valuesIn
  • view
  • when
  • where
  • whereEq
  • without
  • xor
  • xprod
  • zip
  • zipObj
  • zipWith

I think tests would be a great place to start. That way we can be more confident about everything else we do. @somebody1234 , I know you mentioned working on the tests in DT, would you be interested in working on the initial PR for that here? The smaller the better, maybe the framework and then the tests for 2-4 functions?

@somebody1234
Copy link

DT uses their own tool dtslint... but they point to tsd to use elsewhere, should be easy enough to set that up... do note that on DT the tests are written alongside the types so everyone else works on the tests too.

Off-topic but... not too sure about using such an old version of the types, there's a huge amount of work on the types that will need to be carried over... it's probably easier/better to start with the latest version and gradually remove ts-toolbelt

@essenmitsosse
Copy link

There was a discussion with @sandersn from the TypeScript compiler team about an issue we had with dtslint on Discord.

Long story short: While dtslint is quite useful to tests types, the way its done is also quite primitive. Basically all it does is compare the string output of the generated types. But to dtslint string | number and number | string are not compatible, which can a) be super annoying and b) lead to random breaks if the TypeScript compiler changes (in that case it did for a patch release).

Ideally we'd have a tool that uses the actual TypeScript compiler to check types. Is anyone aware of any such tool?

@essenmitsosse
Copy link

Another idea: have a file that has some pseudo TypeScript code, over which we let tsc run as part of the test suit and expect it not to error.

/// Pseudo code
CompareTypes<string, string> // this is fine
CompareTypes<string, number | string> // this throws an error

@somebody1234
Copy link

re: checking types, that would be tsd, no?
they do seem to use the internal isTypeIdenticalTo check (see here)

@somebody1234
Copy link

re: the file with pseudo typescript code, that's good and all but we'd lose the ability to check error cases... which i feel would make the tests a lot less useful

@essenmitsosse
Copy link

@somebody1234 ah yes tsd looks great at first glance. Also you are absolutely right about the error cases. But maybe we can mix different approaches?

On the other hand: what always bothered me was, that you were just checking that there was an error, not where or which. So these tests always seems very prone to be false positive to me.

@somebody1234
Copy link

yeah... too bad neither ts-expect-error nor tsd let you specify specific errors... :(

could maybe fork tsd to support that i guess?

@kedashoe
Copy link
Contributor Author

Now that we have the file structure in place, I think this would be a good next issue to tackle.

Would anyone like to work on it? @essenmitsosse ? @somebody1234 ?

As mentioned in the post at the top, for the initial PR probably just the framework and a small number of functions would be good.

@essenmitsosse
Copy link

essenmitsosse commented May 30, 2022

@kedashoe im still on parental leave for the week but will have a look once I'm back.

@kedashoe
Copy link
Contributor Author

Alright #3283 has been merged in.. anyone have any thoughts on how we should proceed here? Probably making 270ish issues for each function is not the way to go

@valerii15298
Copy link

valerii15298 commented May 31, 2022

We can do checklist for every function.

Ramda has its own tests. As an idea we can change some of them gradually to typescript probably? Some part of them(not sure we can change all due to placeholders and curring).
In this way we can avoid code duplication and even have more confidence in tests. Since if original tests(or even part of them) will pass with typescript it will add quite some confidence in type definitions.

@CrossEye
Copy link
Member

@valerii15298: I think I would have an issue with changing the current tests to TS. Bringing the TS typings in-house is a fine idea, so long as we have people willing to support it. But that won't change the fact that this is a JS library, and it should always remain usable and understandable to those who know JS but not TS.

For separate reasons, I don't want to change the tests overmuch before we reach 1.0, as they should serve as guardrails for the subsequent transition to modern ES in 2.0. After that, I expect to also modernize the tests, and if we can do so in a manner that supports both TS and JS, that would be great, but I haven't given that much thought.

@kedashoe
Copy link
Contributor Author

We can do checklist for every function.

Ya I think this is probably the way to go. I'll update the top post in this thread with a checklist at some point today.

@somebody1234
Copy link

re: porting js tests to ts
it's not viable anyway since the js tests test for all functionality - including functionality we probably don't want in ts.

in other words - the js interface is intended to be robust; the ts interface is intended to prevent you from shooting yourself in the foot (imo)

@kedashoe
Copy link
Contributor Author

ok I've attempted to make a list of all .d.ts files in /types in the top comment. Everyone feel free to start grabbing your favorites to add tests for 😛

I'll make a separate issue for src files missing an accompany type file (not that many).

@valerii15298
Copy link

Maybe then just check js tests and grab from them what is usable for typescript tests. I mean there can be some use cases with js that we wanna ensure work with TS as well🤔

@adispring
Copy link
Member

When we implement ramda apis' types and their tests, we should also focus on practical programing situation, not only just check the types' correctness, but also usable and composable. Otherwise, we only output something looks good, but not so useful for people in real work, such as this issue describes: ramda/types#118.

@younes-io
Copy link

younes-io commented Sep 13, 2022

I want to use splitWhenever in Typescript but I can't :/ Too bad things were done halfway... because according to the docs this function has been added on 0.26.1 and I'm on 0.28

@kedashoe
Copy link
Contributor Author

kedashoe commented Apr 7, 2024

see https://github.com/ramda/types

@kedashoe kedashoe closed this as completed Apr 7, 2024
Typescript Typings automation moved this from In progress to Done Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

7 participants