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

Custom parameters support + Hook conflict fix #182

Merged
merged 4 commits into from
Jun 17, 2024
Merged

Conversation

BugDiver
Copy link
Member

* Parse primitive paramters
* Support custom parameter parser

Signed-off-by: BugDiver <[email protected]>
@BugDiver BugDiver requested a review from zabil June 16, 2024 10:10
Copy link
Member

@zabil zabil left a comment

Choose a reason for hiding this comment

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

Taking a look at this meanwhile I noticed what might be typos. Can you check if it's worth changing the spelling?

docs/index.md Outdated
@@ -309,6 +309,63 @@ String elementId = scenarioStore.get("element-id") as string;

```

### Custom Paramter Parsers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Custom Paramter Parsers
### Custom Parameter Parsers

docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
e2e/tests/PersonParameterParser.ts Outdated Show resolved Hide resolved
gauge-ts/src/RunnerServer.ts Outdated Show resolved Hide resolved
gauge-ts/src/RunnerServer.ts Outdated Show resolved Hide resolved
gauge-ts/src/loaders/ImplLoader.ts Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
Copy link
Member

@zabil zabil left a comment

Choose a reason for hiding this comment

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

Minor comments otherwise all else looks ok!

gauge-ts/tests/loaders/ImplLoaderTests.ts Outdated Show resolved Hide resolved
@Step("This step uses a custom parameter of type Person and value <person>")
public async validatePerson(person: Person) {
assert.equal(person.name, "John");
assert.equal(person.age, 30);
Copy link
Member

Choose a reason for hiding this comment

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

Can we change these to use strictEqual? I noticed that even if the parameter was declared as a number in earlier versions the number is actually passed as string and fails when we do a ===

Copy link
Member Author

@BugDiver BugDiver Jun 17, 2024

Choose a reason for hiding this comment

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

Yes, this should be fixed now, as we are doing primitive conversion. Just to highlight, parameters which are part of a table (string or int or bool) are still treated as string. As of now we are not doing nested conversion.

Copy link
Member

@zabil zabil left a comment

Choose a reason for hiding this comment

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

Two more typos otherwise all good.

gauge-ts/src/loaders/ImplLoader.ts Outdated Show resolved Hide resolved
gauge-ts/src/loaders/ImplLoader.ts Outdated Show resolved Hide resolved
Signed-off-by: BugDiver <[email protected]>
@BugDiver BugDiver added the ReleaseCandidate If a PR is tagged with this label, after merging it will be released label Jun 17, 2024
@gaugebot
Copy link

gaugebot bot commented Jun 17, 2024

@BugDiver Thank you for contributing to gauge-ts. Your pull request has been labeled as a release candidate 🎉🎉.

Merging this PR will trigger a release.

Please bump up the version as part of this PR.

Instructions to bump the version can found at CONTRIBUTING.md

If the CONTRIBUTING.md file does not exist or does not include instructions about bumping up the version, please looks previous commits in git history to see what changes need to be done.

@BugDiver BugDiver merged commit a2a46c5 into main Jun 17, 2024
17 checks passed
@BugDiver BugDiver deleted the custom-paramaters branch June 17, 2024 14:48
@BugDiver BugDiver restored the custom-paramaters branch June 17, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReleaseCandidate If a PR is tagged with this label, after merging it will be released
Development

Successfully merging this pull request may close these issues.

2 participants