test: 3.0.0 initial tests #540
Conversation
check Info Json Schema
check Contact Json Schema asyncapi#539
check License Json Schema asyncapi#539
check Reference Json Schema asyncapi#539
check ReferenceObject Json Schema asyncapi#539
check InfoExtensions Json Schema asyncapi#539
build: extend test cases
|
|
@fmvilas @derberg @dalelane @smoya @char0n @GreenRover @jonaslagoni Check out new tests tldr;
|
|
There is a lot here, so I'll likely have more comments once I've had a proper look through. My initial reaction is mostly to the use of spaces in the file names for the new files - I realise this is very much a personal style/preference thing, so I'm sure it works fine as-is, but I would've preferred these to be hyphenated (such as the files in https://github.com/asyncapi/spec-json-schemas/tree/master/migrations and https://github.com/asyncapi/spec-json-schemas/tree/master/scripts ) or camel-cased (such as the files in https://github.com/asyncapi/spec-json-schemas/tree/master/definitions/3.0.0 ) I'll have a proper look and try and add some more substantive comments later |
|
/ptal |
There was a problem hiding this comment.
Just a word of advice, nobody likes reviewing a 95 file change PR, and some maintainers might even force you to split it up. Also if the new testing setup is unfavored by majority maintainers you could end up having to rewrite it... So small PRs are always preferred!
Other then that, this looks goot to me 👍
However CI does not like it: https://github.com/asyncapi/spec-json-schemas/actions/runs/10109288041/job/27956881619?pr=540
|
I am out with review. This is to much to understand what is the target and how things belong to each other. |
|
|
folks, sorry for delay I understand, that review 95 files may look too much, but it's ordinary tests with basic structure It's not feature implementation, but only tests, don't be afraid 😅 Proposal of this PR is enable basic tests, and tune them on a go Right now, several tests, were disabled until schema fixes All ignored tests, with reasons are described here - #546 |
|
/ptal
|
There was a problem hiding this comment.
Think we waited long enough, thanks for the patience @Pakisan, but as others mentioned, it is indeed a huge PR. But as this is tests and there are no explicit objections, they do not have any effect on the library itself, let's move this forward 🙂 If there are delayed objections we can always revert parts of it.
My initial reaction is mostly to the use of spaces in the file names for the new files - I realise this is very much a personal style/preference thing, so I'm sure it works fine as-is, but I would've preferred these to be hyphenated (such as the files in https://github.com/asyncapi/spec-json-schemas/tree/master/migrations and https://github.com/asyncapi/spec-json-schemas/tree/master/scripts ) or camel-cased (such as the files in https://github.com/asyncapi/spec-json-schemas/tree/master/definitions/3.0.0 )
Regarding the comment from @dalelane I concur, please make an issue after this is merged so we can rectify this, I have a script that can easily do it🙂
|
@Pakisan is there any other followup issues that are needed 🙂? |
|
Next steps:
|
Changes:
All ignored tests, with reasons are described here - #546
closes #539