search

Orange-OpenSource / hurl

Hurl, run and test HTTP requests with plain text.
https://hurl.dev
Apache License 2.0
12.27k stars 476 forks source link

Add actual value type in assert error message #2966

Closed fabricereix closed 1 week ago

fabricereix commented 1 week ago

The type for actual value has been added

11 | retry: {{retry}}
   |          ^^^^^ expecting integer, actual value is string <98>

The error message will be improved using standard 2 lines actual/expected in another PR.

11 | retry: {{retry}}
   |          ^^^^^ actual:   string <98>
   |                expected: int <98>
jcamiel commented 1 week ago

/accept

hurl-bot commented 1 week ago

🕗 /accept is running, please wait for completion.

hurl-bot commented 1 week ago

🔨 Auto rebase from Orange-OpenSource/hurl/master succeeds, Orange-OpenSource/hurl/error/improve-message now embeds these commits:

hurl-bot commented 1 week ago

🕗 /accept is still running, please wait for completion.

hurl-bot commented 1 week ago

❌ Some checks are still failing, please fix them before trying to merge this pull request.

jcamiel commented 1 week ago

@fabricereix I struggled to see why we have different format method, maybe we could rationalize them:

In https://github.com/Orange-OpenSource/hurl/blob/884019c090b413a79d825104f07fea5c5a65cc0c/packages/hurl/src/runner/value.rs#L73 we have a fmt method ( => to_string) on Value.

In the same file, we also have a _typemethod https://github.com/Orange-OpenSource/hurl/blob/884019c090b413a79d825104f07fea5c5a65cc0c/packages/hurl/src/runner/value.rs#L98

which is almost not covered (and is_scalar is also not used).

In https://github.com/Orange-OpenSource/hurl/blob/884019c090b413a79d825104f07fea5c5a65cc0c/packages/hurl/src/runner/predicate.rs#L105

We have display method which seems to have the same semantic as fmt

We also and expected / format which is also a kind of "to_string" method

https://github.com/Orange-OpenSource/hurl/blob/884019c090b413a79d825104f07fea5c5a65cc0c/packages/hurl/src/runner/predicate.rs#L140

fabricereix commented 1 week ago

Yes, I've seen that duplication. But I figured out that this refactoring should belong to another PR. I've just renamed the expected method to format because it was confusing on an actual value.

jcamiel commented 1 week ago

/accept

hurl-bot commented 1 week ago

🕗 /accept is running, please wait for completion.

hurl-bot commented 1 week ago

🔨 Auto rebase from Orange-OpenSource/hurl/master succeeds, Orange-OpenSource/hurl/error/improve-message now embeds these commits:

hurl-bot commented 1 week ago

🕗 /accept is still running, please wait for completion.

hurl-bot commented 1 week ago

✅ Pull request merged and closed by jcamiel with fast forward merge..

# List of commits merged from Orange-OpenSource/hurl/error/improve-message branch into Orange-OpenSource/hurl/master branch: