shortest icon indicating copy to clipboard operation
shortest copied to clipboard

add chained tests

Open crabest opened this issue 1 year ago • 24 comments

For https://github.com/anti-work/shortest/issues/123, I thought of adding chained tests just like a testA inside testB to let testA depends on testB, but with this approach testA cannot depend on another testC unless testC is between testA and testB. As a solution we can let the tests seperated but it can depend on multiple tests no matter the position.

Please let me know your thoughts and feedback.

crabest avatar Dec 24 '24 12:12 crabest

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Dec 24 '24 12:12 CLAassistant

@crabest is attempting to deploy a commit to the Antiwork Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Dec 24 '24 12:12 vercel[bot]

for less confusion, I named it afterTest() that can accept more than one shortest variables

crabest avatar Dec 25 '24 12:12 crabest

I prefer "after", as it's... shorter!

slavingia avatar Dec 25 '24 13:12 slavingia

I said for less confusion because we have after() method for shortest for callback functions image

what do you think

crabest avatar Dec 25 '24 13:12 crabest

Hm where's that coming from, if it's not being imported. But good point, I'll think on it

slavingia avatar Dec 25 '24 13:12 slavingia

Could we overload it? So you could use both?

slavingia avatar Dec 25 '24 13:12 slavingia

we can use both, the "less confusion" just for us humans

crabest avatar Dec 25 '24 13:12 crabest

Let's go with after. I think the simple language is critical for adoption.

slavingia avatar Dec 25 '24 13:12 slavingia

renamed to "after", now it should be good!

crabest avatar Dec 25 '24 13:12 crabest

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
shortest ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 30, 2024 8:32pm

vercel[bot] avatar Dec 26 '24 05:12 vercel[bot]

ideas from internal discussion:

const login = shortest('user can login with email and password')
shortest(login).shortest('user can modify their account-level refund policy from the default of 30-days to no refunds allowed')

const loginAsLawyer = shortest('...')
const loginAsContractor = shortest('...')
const allAppActions = [shortest('send invoice to company'), shortest('view invoices')];
shortest(loginAsLawyer.allAppActions)
shortest(loginAsContractor.allAppActions)

This would make it even shorter.

slavingia avatar Dec 27 '24 18:12 slavingia

let me in those discussions (: for the array part, this would be shorter and more readable.

const allAppActions = shortest(['send invoice to company', 'view invoices'])

crabest avatar Dec 27 '24 18:12 crabest

agreed! sorry, just a daily catchup call we do for our products; I do want to move them to being public! next week! 10am EST Friday

slavingia avatar Dec 27 '24 18:12 slavingia

10am EST Friday, sadly I would still be at work. for those changes, let's iterate in the same pull request? what do you think

crabest avatar Dec 27 '24 18:12 crabest

yep sounds good!

slavingia avatar Dec 27 '24 18:12 slavingia

with those changes, I think let's drop that after()

crabest avatar Dec 27 '24 18:12 crabest

using your example, this is how it will look like now:

const login = 'user can login with email and password'
shortest([login, 'user can modify their account-level refund policy from the default of 30-days to no refunds allowed'])

const loginAsLawyer = '...'
const loginAsContractor = '...'
const allAppActions = ['send invoice to company', 'view invoices'];
shortest([loginAsLawyer, ...allAppActions])
shortest([loginAsContractor, ...allAppActions])

crabest avatar Dec 27 '24 19:12 crabest

Neato! @rmarescu @m2rads et al, happy w/ that?

slavingia avatar Dec 27 '24 19:12 slavingia

my vision is to be like that:

const login = shortest('user can login with email and password')
shortest(login).shortest('user can modify their account-level refund policy from the default of 30-days to no refunds allowed')

const loginAsLawyer = shortest('...')
const loginAsContractor = shortest('...')
const allAppActions = ['send invoice to company', 'view invoices']
shortest(loginAsLawyer).shortest(allAppActions)
shortest(loginAsContractor).shortest(allAppActions)

let me know what do you think

crabest avatar Dec 27 '24 19:12 crabest

I like the shorter one, without the many "shortest"s personally. I was convinced! I can send you the recording of the call if you wish

slavingia avatar Dec 27 '24 19:12 slavingia

i would love to, how

crabest avatar Dec 27 '24 19:12 crabest

DM me on X or email me [email protected] and will send!

slavingia avatar Dec 27 '24 20:12 slavingia

DMed on X

crabest avatar Dec 27 '24 20:12 crabest

Seems like your new changes are not resolving properly.

image

Here's how you can fix it:

When making changes to src/index.ts you'd need to update index.d.ts as well.
We have been dealing with some issues when generating index.d.ts when bundling the package so we implement it manually. Please make sure your new changes are reflected in that file as well.

Here's why this issue happens:

src/index.ts is our source file that contains the actual implementation and gets compiled to JavaScript.

index.d.ts is our declaration file that only contains type definitions and Is used by TypeScript for type checking. It also defines the public API types for consumers.

We need both because the compiled JavaScript (dist/index.js) doesn't contain type information and external users of our package need type definitions for TypeScript support.

I created a new issue: https://github.com/anti-work/shortest/issues/205 to fix it. Please let me know if you have any questions regarding this part.

m2rads avatar Dec 29 '24 12:12 m2rads

Would be good to have a demo using this new feature when you have finished your implementation.

m2rads avatar Dec 29 '24 12:12 m2rads

const login = 'user can login with email and password'
shortest([login, 'user can modify their account-level refund policy from the default of 30-days to no refunds allowed'])

const loginAsLawyer = '...'
const loginAsContractor = '...'
const allAppActions = ['send invoice to company', 'view invoices'];
shortest([loginAsLawyer, ...allAppActions])
shortest([loginAsContractor, ...allAppActions])

the example you used is not implemented, only the example above ^

crabest avatar Dec 29 '24 12:12 crabest

Looks good to me. Would be nice to write some new tests using the new chained tests.

To prepare for release please update your branch and bump the version to 0.3.1. I think this version is appropriate as this PR is a major feature. Also please update changelogs to add your changes.

m2rads avatar Dec 29 '24 21:12 m2rads

0.3.0 imo

slavingia avatar Dec 29 '24 21:12 slavingia

Can we export a test and use it in another test file?

m2rads avatar Dec 29 '24 21:12 m2rads