-
Notifications
You must be signed in to change notification settings - Fork 69
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
test: implement the cucumber conformance tests #238
test: implement the cucumber conformance tests #238
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conformance tests are passing which is great!
But I'm not following why conformance
submodule directory needs to have a golang project inside:
13:50 $ tree conformance/
conformance/
├── cmd
│ └── cloudevents
│ └── main.go
├── features
│ ├── http-protocol-binding.feature
│ └── kafka-protocol-binding.feature
├── go.mod
├── go.sum
├── hack
│ └── presubmit-test.sh
├── LICENSE
├── pkg
│ ├── commands
│ │ ├── commands.go
I'm guess that is because the (updated / common used) conformance spec is there.
If yes, maybe it can be downloaded and / or use the online version ?
It works fine (package.json):
"conformance": "cucumber-js https://raw.githubusercontent.com/cloudevents/conformance/master/features/http-protocol-binding.feature -p default",
13:51 $ npm run conformance
> cloudevents-sdk@2.0.2 conformance /home/hf/code/cloudevents/sdk-javascript
> cucumber-js https://raw.githubusercontent.com/cloudevents/conformance/master/features/http-protocol-binding.feature -p default
@http
Feature: HTTP protocol binding
@http
Scenario Outline: Binary content mode (application/json)
Given HTTP Protocol Binding is supported
Given an HTTP request
"""
POST /someresource HTTP/1.1
...
@helio-frota that's a good suggestion. I didn't realize you could provide a URL. Definitely better than pulling in a submodule |
@helio-frota I have not been able to make your suggestion work. Unclear why. 🤷♂️ |
@lance that was a false positive : ( the submodule was here, and when running pointing to a remote file, the cucumber falls back to the local file in the disk. Sorry. But to try to remedy that I have created a small package called
It is working least. |
This commit adds cucumber-js conformance steps and includes the cucumber tests in 'npm test'. Signed-off-by: Lance Ball <lball@redhat.com>
3307be8
to
f639fb6
Compare
@cloudevents/sdk-javascript-maintainers PTAL @helio-frota thanks for the suggestions - done. |
Ping @cloudevents/sdk-javascript-maintainers ptal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reasoning for moving the tests to an integration directory? Will the move mess with the merging of #233 since those tests are being updated there?
@lholmquist it's mainly to keep the integration tests separate from the conformance steps that are now in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welp, LGTM then
Fixes: #71
Items remaining
git submodule
to pull in the features from cloudevents/spec