Skip to content

Conversation

@kakserpom
Copy link
Contributor

Description

Implements #493

@coveralls
Copy link

coveralls commented Dec 9, 2025

Pull Request Test Coverage Report for Build 20234104119

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 43 of 56 (76.79%) changed or added relevant lines in 4 files are covered.
  • 32 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.8%) to 36.015%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/types/array/conversions/btree_set.rs 12 16 75.0%
src/types/array/conversions/hash_set.rs 12 16 75.0%
src/types/array/array_key.rs 5 10 50.0%
Files with Coverage Reduction New Missed Lines %
src/types/array/array_key.rs 2 53.06%
crates/macros/src/lib.rs 30 26.92%
Totals Coverage Status
Change from base Build 20110536661: 0.8%
Covered Lines: 1652
Relevant Lines: 4587

💛 - Coveralls

@kakserpom kakserpom force-pushed the 493_feat_sets_in_arguments branch 3 times, most recently from be00b05 to 3a743b0 Compare December 9, 2025 15:31
@kakserpom kakserpom force-pushed the 493_feat_sets_in_arguments branch from 3a743b0 to 455b192 Compare December 9, 2025 15:34
@Xenira
Copy link
Member

Xenira commented Dec 9, 2025

Thanks. Can you add unit tests?

@kakserpom
Copy link
Contributor Author

I am not sure what should be tested here.

Copy link
Member

@ptondereau ptondereau left a comment

Choose a reason for hiding this comment

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

Thanks for the implementation! Just some comments.

@ptondereau
Copy link
Member

ptondereau commented Dec 10, 2025

I am not sure what should be tested here.

for example:

#[test]
fn test_hash_table_try_from_hash_set() {
      Embed::run(|| {
          let mut set = HashSet::new();
          set.insert("one");
          let ht: ZBox<ZendHashTable> = set.try_into().unwrap();
          assert_eq!(ht.len(), 1);
          assert!(ht.get(0).is_some());
      });
}

@kakserpom kakserpom force-pushed the 493_feat_sets_in_arguments branch 2 times, most recently from 93b1a3c to 99a40db Compare December 10, 2025 19:51
@kakserpom kakserpom force-pushed the 493_feat_sets_in_arguments branch from 99a40db to d36bd8d Compare December 12, 2025 16:44
@kakserpom
Copy link
Contributor Author

@ptondereau @Xenira take a look pls =)

Copy link
Member

@ptondereau ptondereau left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@ptondereau ptondereau merged commit 146e92e into extphprs:master Dec 15, 2025
62 of 63 checks passed
This was referenced Dec 14, 2025
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