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

fix: this patches the strEndsWith test case #47

Merged
merged 2 commits into from
Jan 31, 2023

Conversation

sighphyre
Copy link
Member

The strEndsWith test case was previously checking that @some-email.com ends with @some-email.com. This means that an incorrectly implemented SDK could check that the context property ended with the constraint property and still pass the specifications.

This patches that behaviour, meaning that dependent SDKs will now fail that test case correctly if the implementation is incorrect.

…he string ends with rather than allowing the inverse
@ivarconr ivarconr self-requested a review January 25, 2023 19:09
Copy link
Member

@ivarconr ivarconr left a comment

Choose a reason for hiding this comment

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

Maybe also change the test cases to use actual email addresses, to be a more realistic example?

"description": "F3.endsWith should be enabled",
"context": {
"properties": {
"email": "@some-email.com"
}
},
"toggleName": "F3.endsWith",
"expectedResult": true
},
{
"description": "F3.endsWith should be disabled when casing is incorrect",
"context": {
"properties": {
"email": "@some-EMAIL.com"
}
},
"toggleName": "F3.endsWith",
"expectedResult": false
},
{
"description": "F3.endsWith.ignoringCase should be enabled",
"context": {
"properties": {
"email": "@SOME-EMAIL.com"
}
},
"toggleName": "F3.endsWith.ignoringCase",
"expectedResult": true
},
{
"description": "F3.endsWith should be disabled",
"context": {
"properties": {
"email": "@another-email.com"
}
},
"toggleName": "F3.endsWith",
"expectedResult": false
},
{
"description": "F4.contains should be enabled",
"context": {
"properties": {
"email": "@some-email.com"
}
},
"toggleName": "F4.contains",
"expectedResult": true
},
{
"description": "F4.contains should be disabled",
"context": {
"properties": {
"email": "@another.com"
}
},
"toggleName": "F4.contains",
"expectedResult": false
},
{
"description": "F4.contains.inverted should be enabled",
"context": {
"properties": {
"email": "@another.com"
}

@sighphyre
Copy link
Member Author

Maybe also change the test cases to use actual email addresses, to be a more realistic example?

Good suggestion! I've updated the tests to have a somewhat realistic email

@sighphyre sighphyre requested a review from ivarconr January 26, 2023 07:17
@sighphyre sighphyre merged commit bfe9b49 into main Jan 31, 2023
@sighphyre sighphyre deleted the fix/patch-ends-with-test-case branch January 31, 2023 08:35
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.

2 participants