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

FTRACK UserId Submodule: adding more tests for the get ID methods in pbjs #8737

Conversation

ftxmoJason
Copy link
Contributor

Type of change

  • Refactoring (no functional changes, no api changes)

Description of change

  • after making changes and adding tests for pbjs.getUserIdsAsEids(), we realized we also lacked tests for pbjs.getUserIdsAsync() and pbjs.getUserIds()

Other information

@patmmccann patmmccann changed the title FTRACK: adding more tests for the get ID methods in pbjs FTRACK UserId Submodule: adding more tests for the get ID methods in pbjs Jul 25, 2022
@patmmccann patmmccann requested a review from smenzer July 25, 2022 19:51
@patmmccann
Copy link
Collaborator

We love extra test PRs, ty!

@ftxmoJason ftxmoJason marked this pull request as ready for review July 26, 2022 15:23
@ftxmoJason
Copy link
Contributor Author

@patmmccann I think this is ready to roll.

Do we think it would be helpful if there was a comment in the User Id submodule build directions asking folks to include tests for pbjs.getUserIdsAsync(), pbjs.getUserIds() and pbjs.getUserIdsAsEids()? Knowing what I know now, it feels pretty integral to have the responses from those methods dialed in.

@patmmccann
Copy link
Collaborator

It seems wise if the return structure is complicated, like yours or @Nick-Merkle

@ftxmoJason
Copy link
Contributor Author

@patmmccann Added a PR to the documentation prebid/prebid.github.io#3929

Copy link
Collaborator

@smenzer smenzer left a comment

Choose a reason for hiding this comment

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

LGTM

@smenzer smenzer merged commit d22f149 into prebid:master Jul 26, 2022
JacobKlein26 pushed a commit to nextmillenniummedia/Prebid.js that referenced this pull request Feb 9, 2023
…pbjs (prebid#8737)

* FTRACK: adding more tests for the get ID methods in pbjs

* FTRACK: making the tests a little DRYer

* FTRACK: cleaning up the tests

* FTRACK: cleaning up some comments

Co-authored-by: Jason Lydon <jason.lydon@flashtalking.com>
jorgeluisrocha pushed a commit to jwplayer/Prebid.js that referenced this pull request May 23, 2023
…pbjs (prebid#8737)

* FTRACK: adding more tests for the get ID methods in pbjs

* FTRACK: making the tests a little DRYer

* FTRACK: cleaning up the tests

* FTRACK: cleaning up some comments

Co-authored-by: Jason Lydon <jason.lydon@flashtalking.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants