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

feat: tck 100% coverage #284

Closed

Conversation

KhudaDad414
Copy link
Member

Description

  • I am planning to integrate tck into parser-js. before doing that we have to make sure that tck has 100% coverage in this repository.
    Related issue(s)
    Resolves 100% coverage for tck #283

@KhudaDad414
Copy link
Member Author

KhudaDad414 commented Apr 11, 2021

@derberg can you confirm if this is a good approach to add 100% coverage for tck? after your confirmation, I can add validation for 100% of tck files (I think).

Corrected some typo.
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@derberg
Copy link
Member

derberg commented Apr 12, 2021

@KhudaDad414 adding 100% coverage for tck would be awesome, but it should not happen in one run but one test case after another to not overcomplicate review process and have it smooth.

big challenges ahead:

  • sometimes we validate and throw error on something that actually should just be a warning and not error, but problem is we do not have a concept of warning in the parser, that warns but doesn't fail parsing -> Improve error handling in parser #119 (comment). Fixing this first will block you from having a 100% coverage
  • majority of failures you can see here https://asyncapi.github.io/tck/asyncapi-parser_js_detailed_report.html is that test cases have not only simple documents but also documents with bindings. Problem is that the current spec doesn't require providing a json schema for binding, and therefore there is no easy way to do validation. In my opinion, handling types definitions in our code is not the best approach, hard to maintain, especially that we expect bindings and their releases are managed by different community members that not necessarily care about JS parser. I n my opinion we should rather make sure we have JSON Schemas for bindings even if it is not required by the spec

@derberg
Copy link
Member

derberg commented Apr 14, 2021

@KhudaDad414 we clarified a bit, have a look at asyncapi/spec#507. Best approach is to create json schema files for bindings

@KhudaDad414
Copy link
Member Author

KhudaDad414 commented Apr 14, 2021

@derberg thanks!

@KhudaDad414 KhudaDad414 deleted the feat-tck-100%-coverage branch September 6, 2021 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

100% coverage for tck
2 participants