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

future: add meta 3rd: busted #1556

Merged
merged 17 commits into from
Sep 22, 2022

Conversation

fesily
Copy link
Contributor

@fesily fesily commented Sep 16, 2022

No description provided.

@carsakiller
Copy link
Collaborator

This is one I actually use and have my own documentation for that I can add to this tomorrow - it is getting late 🙂

@carsakiller carsakiller marked this pull request as draft September 16, 2022 04:22
@carsakiller carsakiller self-requested a review September 16, 2022 04:23
@carsakiller carsakiller added the documentation Has to do with documentation either in the wiki or in the repo label Sep 16, 2022
@carsakiller
Copy link
Collaborator

Thanks for opening the PR and getting the ball rolling on this 👍

@fesily
Copy link
Contributor Author

fesily commented Sep 16, 2022

The documentation provided by the author of busted is too simple and I cannot provide too much detail. If you have something better to add, that would be great!

@serg3295

This comment was marked as resolved.

@fesily

This comment was marked as resolved.

@serg3295

This comment was marked as resolved.

@carsakiller carsakiller force-pushed the future--add-meta-3rd--busted branch from 21594fa to 270f0ae Compare September 16, 2022 13:33
@carsakiller
Copy link
Collaborator

I am not able to use this file() function... where did you find it?

@fesily
Copy link
Contributor Author

fesily commented Sep 17, 2022

I am not able to use this file() function... where did you find it?

I read the source code, he provides this api, but should not use this api, unless you are developing a test tool

@carsakiller
Copy link
Collaborator

There is a LOT that is undocumented under busted and luassert. Trying my best to cover everything...

- Also removed unnecessary unique type for stub instances
@carsakiller
Copy link
Collaborator

Still need to get to matchers, but I think I got almost everything.

@carsakiller

This comment was marked as resolved.

@flrgh
Copy link
Contributor

flrgh commented Sep 17, 2022

Oh man, this is a very challenging one to annotate given all of the metatable magic. Props to y'all for taking it on!

@PerMalmberg

This comment was marked as resolved.

@carsakiller

This comment was marked as resolved.

@carsakiller carsakiller removed their request for review September 18, 2022 15:06
@carsakiller carsakiller self-assigned this Sep 18, 2022
@carsakiller
Copy link
Collaborator

I did not realize that there is SO many aliases. These are all valid and perform the same assertion:

assert.number
assert.Number
assert.is.Number
assert.is.number
assert.is_number
assert.is_Number

@PerMalmberg
Copy link

I did not realize that there is SO many aliases. These are all valid and perform the same assertion:

assert.number
assert.Number
assert.is.Number
assert.is.number
assert.is_number
assert.is_Number

Looks like something that could be generated in script.

@fesily
Copy link
Contributor Author

fesily commented Sep 20, 2022

At the moment it is not possible to derive all api, so I made a trade-off and added only the most simple and intuitive api.

Copy link
Contributor Author

@fesily fesily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not move the luassert file to the top-level directory, it will be invalid

@carsakiller
Copy link
Collaborator

My favourite is luassert.is_not_no_error 😆

@fesily fesily marked this pull request as ready for review September 22, 2022 06:01
@fesily
Copy link
Contributor Author

fesily commented Sep 22, 2022

I think it is now ready to be merged, thanks @carsakiller

@sumneko
Copy link
Collaborator

sumneko commented Sep 22, 2022

Need to resolve tests.

EDIT: It seems that my CI fails to receive email notifications is broken.

@carsakiller
Copy link
Collaborator

Nicely done resolving the nesting! 👍

@carsakiller carsakiller merged commit 3ddd84c into LuaLS:master Sep 22, 2022
@PerMalmberg
Copy link

Is there a release planned that includes this? Would love to start using it.

@carsakiller
Copy link
Collaborator

It will be in the next release which will probably be out in the next couple days 🙂

@PerMalmberg
Copy link

Awesome :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Has to do with documentation either in the wiki or in the repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants