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

remove bidmanager from rubicon tests #1671

Merged

Conversation

snapwich
Copy link
Collaborator

@snapwich snapwich commented Oct 9, 2017

Type of change

  • Refactoring (no functional changes, no api changes)

Description of change

Removing all references of the bidManager from the rubicon tests to be 1.0 compatible.

@jaiminpanchal27 jaiminpanchal27 self-assigned this Oct 9, 2017
var sizesBidderRequest = clone(bidderRequest);
sizesBidderRequest.bids[0].sizes = [[620, 250], [300, 251]];

sandbox.stub(spec, 'buildRequests');

rubiconAdapter.callBids(sizesBidderRequest);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test can be easily tested by using spec.isBidRequestValid and spec.buildRequests. Call to callBids can be removed.

Same goes for other tests in for request test suite. They can be tested using spec functions.

Copy link
Collaborator

@jaiminpanchal27 jaiminpanchal27 left a comment

Choose a reason for hiding this comment

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

Also, Is this tested against prebid-1.0 branch ?

@snapwich
Copy link
Collaborator Author

snapwich commented Oct 9, 2017

Updated. Also tested against 1.0 branch and everything passed there as well.

Copy link
Collaborator

@jaiminpanchal27 jaiminpanchal27 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -191,39 +153,22 @@ describe('the rubicon adapter', () => {
});
});

describe('callBids implementation', () => {
describe('buildRequests implementation', () => {
let rubiconAdapter;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can get rid of the rubiconAdapter here, since your tests work on the spec

@@ -1005,7 +816,6 @@ describe('the rubicon adapter', () => {
beforeEach(() => {
// monitor userSync registrations
userSyncStub = sandbox.stub(userSync, 'registerSync');
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you relying on this stub? If you're just testing the response of getUserSyncs, I don't think you should have to.

@@ -128,43 +127,6 @@ describe('the rubicon adapter', () => {
sandbox.restore();
Copy link
Contributor

Choose a reason for hiding this comment

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

sandbox probably shouldn't need to useFakeServer anymore. Also it looks like the adUnit (a few dozen lines above this comment, so github won't let me put a comment there) is unused now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

those have been removed

@dbemiller dbemiller merged commit a907a4a into prebid:master Oct 11, 2017
outoftime pushed a commit to Genius/Prebid.js that referenced this pull request Oct 18, 2017
* tag '0.31.0' of https://github.com/prebid/Prebid.js: (54 commits)
  Fix for prebid#1628 (allowing standard bidCpmAdjustment) (prebid#1645)
  Prebid 0.31.0 Release
  Support native click tracking (prebid#1691)
  Initial commit for video support for pbs (prebid#1706)
  Fixes: Immediate adapter response may end auction (prebid#1690)
  Rubicon feature/s2s test module (prebid#1678)
  Renaming of "huddledmasses" adapter into colossusssp (prebid#1701)
  Don't set non-object configurations (prebid#1704)
  Update JSDoc for `pbjs.enableAnalytics` (prebid#1565)
  Add ad units event (prebid#1702)
  AppnexusAst adapter: logging error message from endpoint (prebid#1697)
  AppnexusAst bidadapter markdown file (prebid#1696)
  Change Default Content-Type for POST Requests to 'application/json' (prebid#1681)
  Code improvement for trustx adapter (prebid#1673)
  PulsePoint Lite adapter - Enabling Sync pixel (prebid#1686)
  Update spotx video adapter to set the spotx_ad_key used in DFP (prebid#1614)
  Fix broken AOL mobile endpoint secure bid requests (prebid#1684)
  Fix adapter tests that hardcoded pbjs. (prebid#1666)
  no longer attaching gpt slots to adUnits, which breaks utils.cloneJson(adUnit) (prebid#1676)
  remove bidmanager from rubicon tests (prebid#1671)
  ...
mattpr pushed a commit to mattpr/Prebid.js that referenced this pull request Oct 31, 2017
* remove bidmanager from rubicon tests

* updated rubicon tests to not use callBids

* remove no longer used artifacts from rubiconBidAdapter_spec
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* remove bidmanager from rubicon tests

* updated rubicon tests to not use callBids

* remove no longer used artifacts from rubiconBidAdapter_spec
@robertrmartinez robertrmartinez deleted the rubicon-bidManager-out-of-tests branch July 5, 2023 19:45
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.

4 participants