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

[WIP] AppEngine: Implement property-based tests for cast_value functions #980

Draft
wants to merge 4 commits into
base: release-1.2
Choose a base branch
from

Conversation

Hibe7
Copy link
Contributor

@Hibe7 Hibe7 commented Jul 18, 2024

Implement property-based tests for cast_value functions.

What this PR does / why we need it:

  • Implement property-based tests for cast_value functions.

Special notes for your reviewer:

  • The review of this PR can proceed after this PR gets accepted.

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.05%. Comparing base (c737fc5) to head (d50b08b).

Additional details and impacted files
@@               Coverage Diff               @@
##           release-1.2     #980      +/-   ##
===============================================
- Coverage        69.62%   64.05%   -5.57%     
===============================================
  Files              264      208      -56     
  Lines             7097     5659    -1438     
===============================================
- Hits              4941     3625    -1316     
+ Misses            2156     2034     -122     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CHANGELOG.md Show resolved Hide resolved
@shinnokdisengir shinnokdisengir self-requested a review July 18, 2024 09:25
Copy link
Collaborator

@Annopaolo Annopaolo left a comment

Choose a reason for hiding this comment

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

Also, some nitpicking on the PR title: this PR introduces property-based tests for value casts, not for the whole AppEngine as the title seems to suggest

shinnokdisengir and others added 4 commits July 22, 2024 10:29
Changed
- using exandra -- waiting for database project
- implemented jwt + public/private key
- wip group tests
- squashed all failure
- polishing
- removed one_of_constant -> member_of
- splitted tests
- WIP device generator
- completed device generator...but using opaque data (TODO change it)
- basic datetime generator
- WIP device generator
- using Keyword.validate!
- WIP insert into db, IPNET error
- completed device WIP id doesn't match
- completed base tests device
- polishing
- renamed realm -> keyspace
- completed group generator + setup + case
- wip group test (group_name "!" doesn't work)
- added encoded_id field to device test
- unlinked Conn <-> Database cases
- fixed realm name
- fixed valid group name test
- added already exists
- polished
- removed common.ex

Signed-off-by: Gabriele Ghio <gabriele.ghio@secomind.com>
Signed-off-by: Ismet Softic <ismet.softic@secomind.com>
Signed-off-by: Ismet Softic <ismet.softic@secomind.com>
Implement property-based tests for `cast_value` functions.

Signed-off-by: Ismet Softic <ismet.softic@secomind.com>
@Hibe7 Hibe7 force-pushed the implementPropertyBasedTests branch from 6168b87 to d50b08b Compare July 25, 2024 14:06
@Hibe7 Hibe7 changed the title [WIP] AppEngine: Implement property-based tests [WIP] AppEngine: Implement property-based tests for cast_value functions Jul 25, 2024
end

def invalid_binaryblob_array() do
list_of(invalid_binaryblob(), min_length: 1, max_length: 10)
Copy link
Contributor

@shinnokdisengir shinnokdisengir Aug 5, 2024

Choose a reason for hiding this comment

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

invalid_binaryblob() |> list_of would be better? - (generators of invalid entites must be removed, see below)

end

def invalid_datetime() do
StreamData.one_of([
Copy link
Contributor

Choose a reason for hiding this comment

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

use use ExUnitProperties like file above

date_time!(min, max)
end

def invalid_datetime() do
Copy link
Contributor

@shinnokdisengir shinnokdisengir Aug 5, 2024

Choose a reason for hiding this comment

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

generators must produce valid entities. If you want to create a mistake, use a pattern similar to apps/astarte_appengine_api/test/astarte_appengine_api/v2_group_test.exs

Copy link
Contributor

Choose a reason for hiding this comment

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

(If you need to reuse an invalid generator in more than one test, you could implement at the beginning of the test file)

alias Astarte.Test.Generators.DateTime, as: DateTimeGenerator
alias Astarte.Test.Generators.Number, as: NumberGenerator
alias Astarte.Test.Generators.String, as: StringGenerator

Copy link
Contributor

Choose a reason for hiding this comment

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

@moduletag :v2
@moduletag :interface # or interface_value? It's the context

Copy link
Contributor

Choose a reason for hiding this comment

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

the v2 tag is useful when we want to test only new version - property based - (ex. mix test --only v2)

end

property "Valid binary blob values are correctly casted" do
check all blob <- StreamData.binary() do
Copy link
Contributor

Choose a reason for hiding this comment

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

remove unnecessary StreamData. - we are using use ExUnitProperties

end

def longinteger_array() do
StreamData.one_of([
Copy link
Contributor

Choose a reason for hiding this comment

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

My bad, I've forgotten to remove all StreamData. from this file. Please fix both of code

Copy link
Contributor

Choose a reason for hiding this comment

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

Anyway, I don't like this approach: using two different function for integer and strings would be better. Then, you could create a mixed one (but it's odd, I prefer to maintain them seperately).
Also, look this code:

parsed_integers =
        Enum.map(long_integer_array, fn
          value when is_integer(value) -> value
          value when is_binary(value) -> String.to_integer(value)
        end)

PS: could you put these functions into a array.ex file?

date_time!(min, max)
end

def invalid_datetime() do
Copy link
Contributor

Choose a reason for hiding this comment

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

(If you need to reuse an invalid generator in more than one test, you could implement at the beginning of the test file)

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.

4 participants