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

[Tests] Add test Coverage for Symbol class #957

Closed
JckXia opened this issue Mar 31, 2021 · 7 comments
Closed

[Tests] Add test Coverage for Symbol class #957

JckXia opened this issue Mar 31, 2021 · 7 comments
Labels

Comments

@JckXia
Copy link
Member

JckXia commented Mar 31, 2021

class methods
Covered #972 static Symbol New(napi_env env, const std::string& description)
Covered #972 static Symbol New(napi_env env, napi_value description)
Covered #972 Symbol()
Covered #972 Symbol(napi_env env, napi_value description)
Covered #972 Symbol(napi_env env, Napi::String description)
Covered #972 Symbol(napi_env env, const std::string& description)
Covered #972 Symbol(napi_env env , const char* description)
Covered #972 Symbol::Wellknown(napi_env env, const std::string& name)
@JckXia JckXia changed the title [Tests] Add tests for Symbol class [Tests] Add test Coverage for Symbol class Mar 31, 2021
@JckXia
Copy link
Member Author

JckXia commented Mar 31, 2021

Will work on adding these tests soon.

@KevinEady
Copy link
Contributor

I would add Symbol::WellKnown as well. There seems to be some discrepancy: #919

@NickNaso NickNaso added the test label Apr 1, 2021
@JckXia
Copy link
Member Author

JckXia commented Apr 15, 2021

@KevinEady Hey Kevin. For the Symbol::Wellknown function, should it return a symbol that we have registered via Symbol.for? In that if I call Symbol.for("testSymbol") on js side, I should receive the same symbol back with a call to Symbol::Wellknown("testSymbol") from Cpp? (Right now it will return undefined as stated in #919)

@ThomasWeinert
Copy link

I would suggest adding a separate Symbol::For(napi_env env, const std::string& name) for fetching global symbols.

@github-actions
Copy link
Contributor

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Jul 16, 2021
@NickNaso NickNaso removed the stale label Jul 16, 2021
@NickNaso
Copy link
Member

NickNaso commented Jul 16, 2021

Hi @JckXia ,
the PR #972 has been landed. Could you provide to update the matrix of methods that needs to be tested? In case you already covered all the ceses please provide to close the issue.

@JckXia
Copy link
Member Author

JckXia commented Jul 16, 2021

@NickNaso Thank you for the reminder! I marked the test cases as covered and the issue is ready to be closed

@JckXia JckXia closed this as completed Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants