Skip to content

Commit

Permalink
Refactoring code as per comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Amit3200 committed Oct 25, 2023
1 parent bd070e8 commit 6c9f693
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 41 deletions.
6 changes: 3 additions & 3 deletions packages/webdriver-utils/src/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export default class Driver {
this.passedCapabilities = passedCapabilities;
}

requestPostOptions(command) {
static requestPostOptions(command) {
return {
method: 'POST',
headers: {
Expand Down Expand Up @@ -53,7 +53,7 @@ export default class Driver {
if (!command.script.includes('browserstack_executor')) {
command.script = `/* percy_automate_script */ \n ${command.script}`;
}
const options = this.requestPostOptions(command);
const options = Driver.requestPostOptions(command);
const baseUrl = `${this.executorUrl}/session/${this.sessionId}/execute/sync`;
const response = JSON.parse((await request(baseUrl, options)).body);
return response;
Expand All @@ -72,7 +72,7 @@ export default class Driver {
}

async findElement(using, value) {
const options = this.requestPostOptions({ using, value });
const options = Driver.requestPostOptions({ using, value });
const baseUrl = `${this.executorUrl}/session/${this.sessionId}/element`;
const response = JSON.parse((await request(baseUrl, options)).body);
return response.value;
Expand Down
26 changes: 12 additions & 14 deletions packages/webdriver-utils/src/providers/genericProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,10 @@ export default class GenericProvider {
this.debugUrl = null;
this.header = 0;
this.footer = 0;
this.pageYShiftFactor = 0;
this.statusBarHeight = 0;
this.pageXShiftFactor = 0;
this.scrollXFactor = 0;
this.scrollYFactor = 0;
this.currentOs = null;
this.pageYShiftFactor = 0;
this.currentOperatingSystem = null;
}

addDefaultOptions() {
Expand Down Expand Up @@ -94,12 +93,8 @@ export default class GenericProvider {
const tiles = await this.getTiles(this.header, this.footer, fullscreen);
log.debug(`[${name}] : Tiles ${JSON.stringify(tiles)}`);

const scrollFactors = await this.driver.executeScript({ script: 'return [parseInt(window.scrollX * window.devicePixelRatio), parseInt(window.scrollY * window.devicePixelRatio)];', args: [] });
this.scrollXFactor = scrollFactors.value[0];
this.scrollYFactor = scrollFactors.value[1];
this.currentOs = tag.osName;
this.pageYShiftFactor = tag.osName === 'iOS' ? tiles.tiles[0].statusBarHeight : (tiles.tiles[0].statusBarHeight - scrollFactors.value[1]);
this.pageXShiftFactor = tag.osName === 'iOS' ? 0 : (-scrollFactors.value[0]);
this.currentOperatingSystem = tag.osName;
this.statusBarHeight = tiles.tiles[0].statusBarHeight;

const ignoreRegions = await this.findRegions(
ignoreRegionXpaths, ignoreRegionSelectors, ignoreRegionElements, customIgnoreRegions
Expand Down Expand Up @@ -203,10 +198,13 @@ export default class GenericProvider {
];
}

updateYFactor(location) {
if (this.currentOs === 'iOS') {
async updatePageShiftFactor(location) {
const scrollFactors = await this.driver.executeScript({ script: 'return [parseInt(window.scrollX * window.devicePixelRatio), parseInt(window.scrollY * window.devicePixelRatio)];', args: [] });
this.pageYShiftFactor = this.currentOperatingSystem === 'iOS' ? this.statusBarHeight : (this.statusBarHeight - scrollFactors.value[1]);
this.pageXShiftFactor = this.currentOperatingSystem === 'iOS' ? 0 : (-scrollFactors.value[0]);
if (this.currentOperatingSystem === 'iOS') {
if (location.y === 0) {
this.pageYShiftFactor += (-this.scrollYFactor);
this.pageYShiftFactor += (-scrollFactors.value[1]);
}
}
}
Expand All @@ -218,7 +216,7 @@ export default class GenericProvider {
const size = { height: rect.height, width: rect.width };
// Update YFactor Element is not visible in viewport
// In case of iOS if the element is not visible in viewport it gives 0,0 as coordinate.
this.updateYFactor(location);
await this.updatePageShiftFactor(location);
const coOrdinates = {
top: Math.floor(location.y * scaleFactor) + this.pageYShiftFactor,
bottom: Math.ceil((location.y + size.height) * scaleFactor) + this.pageYShiftFactor,
Expand Down
2 changes: 1 addition & 1 deletion packages/webdriver-utils/test/driver.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ describe('Driver', () => {
body: JSON.stringify(command)
};
it('returns post options', () => {
expect(driver.requestPostOptions(command)).toEqual(expectedResponse);
expect(Driver.requestPostOptions(command)).toEqual(expectedResponse);
});
});
});
56 changes: 33 additions & 23 deletions packages/webdriver-utils/test/providers/genericProvider.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,22 +167,18 @@ describe('GenericProvider', () => {
}],
domInfoSha: 'mock-dom-sha'
};
const scrollFactors = { value: [0, 10] };
getTagSpy = spyOn(GenericProvider.prototype, 'getTag').and.returnValue(Promise.resolve(desktopTag));
getTilesSpy = spyOn(GenericProvider.prototype, 'getTiles').and.returnValue(Promise.resolve(desktopTiles));
spyOn(DesktopMetaData.prototype, 'windowSize')
.and.returnValue(Promise.resolve({ width: 1920, height: 1080 }));
spyOn(Driver.prototype, 'executeScript')
.and.returnValue(scrollFactors);
});

it('calls correct funcs', async () => {
genericProvider = new GenericProvider('123', 'http:executorUrl', { platform: 'win' }, {}, 'local-poc-poa', 'staging-poc-poa', {});
await genericProvider.createDriver();
let res = await genericProvider.screenshot('mock-name', {});
expect(getTagSpy).toHaveBeenCalledTimes(1);
expect(genericProvider.pageYShiftFactor).toEqual(-10);
expect(genericProvider.pageXShiftFactor).toEqual(-0);
expect(genericProvider.statusBarHeight).toEqual(0);
expect(getTilesSpy).toHaveBeenCalledOnceWith(0, 0, false);
expect(res).toEqual({
name: 'mock-name',
Expand Down Expand Up @@ -240,8 +236,7 @@ describe('GenericProvider', () => {
await genericProvider.createDriver();
let res = await genericProvider.screenshot('mock-name', {});
expect(iOSGetTagSpy).toHaveBeenCalledTimes(1);
expect(genericProvider.pageYShiftFactor).toEqual(132);
expect(genericProvider.pageXShiftFactor).toEqual(0);
expect(genericProvider.statusBarHeight).toEqual(132);
expect(iOSGetTilesSpy).toHaveBeenCalledOnceWith(0, 0, false);
expect(res).toEqual({
name: 'mock-name',
Expand Down Expand Up @@ -273,41 +268,50 @@ describe('GenericProvider', () => {
});
});

describe('updateYFactor', () => {
describe('updatePageShiftFactor', () => {
let provider;

describe('When iOS', () => {
beforeEach(() => {
beforeEach(async () => {
provider = new GenericProvider('123', 'http:executorUrl', { platform: 'win' }, {});
provider.currentOs = 'iOS';
provider.scrollYFactor = 10;
await provider.createDriver();
spyOn(Driver.prototype, 'executeScript').and.returnValue({ value: [0, 10] });
provider.currentOperatingSystem = 'iOS';
provider.pageYShiftFactor = 0;
provider.statusBarHeight = 0;
});

it('should update pageYShiftFactor for iOS when location.y is 0', () => {
provider.updateYFactor({ y: 0 });
expect(provider.pageYShiftFactor).toBe(-provider.scrollYFactor);
it('should update pageYShiftFactor for iOS when location.y is 0', async () => {
await provider.updatePageShiftFactor({ y: 0 });
expect(provider.pageYShiftFactor).toBe(-10);
});

it('should not update pageYShiftFactor for iOS when location.y is not 0', () => {
it('should not update pageYShiftFactor for iOS when location.y is not 0', async () => {
// Location.y is not 0
provider.updateYFactor({ y: 5 });
await provider.updatePageShiftFactor({ y: 5 });
expect(provider.pageYShiftFactor).toBe(0);
});
});

describe('When Other', () => {
beforeEach(() => {
beforeEach(async () => {
provider = new GenericProvider('123', 'http:executorUrl', { platform: 'win' }, {});
provider.currentOs = 'Android';
provider.scrollYFactor = 10;
await provider.createDriver();
provider.currentOperatingSystem = 'Android';
provider.pageYShiftFactor = 0;
});

it('should not update pageYShiftFactor for non-iOS platforms', () => {
provider.updateYFactor({ y: 0 });
it('should not update pageYShiftFactor for non-iOS platforms', async () => {
spyOn(Driver.prototype, 'executeScript').and.returnValue({ value: [0, 0] });
await provider.updatePageShiftFactor({ y: 0 });
expect(provider.pageYShiftFactor).toBe(0);
});

it('should update pageYShiftFactor for non-iOS platforms accordingly if scrolled', async () => {
spyOn(Driver.prototype, 'executeScript').and.returnValue({ value: [0, 10] });
await provider.updatePageShiftFactor({ y: 0 });
expect(provider.pageYShiftFactor).toBe(-10);
});
});
});

Expand All @@ -316,11 +320,14 @@ describe('GenericProvider', () => {
let mockLocation = { x: 10, y: 20, width: 100, height: 200 };

describe('When on Tile 0', () => {
beforeEach(() => {
beforeEach(async () => {
// mock metadata
provider = new GenericProvider('123', 'http:executorUrl', { platform: 'win' }, {});
await provider.createDriver();
spyOn(DesktopMetaData.prototype, 'devicePixelRatio')
.and.returnValue(1);
spyOn(Driver.prototype, 'executeScript')
.and.returnValue({ value: [0, 0] });
spyOn(Driver.prototype, 'rect').and.returnValue(Promise.resolve(mockLocation));
});

Expand All @@ -343,11 +350,14 @@ describe('GenericProvider', () => {
});

describe('When on Tile 1', () => {
beforeEach(() => {
beforeEach(async () => {
// mock metadata
provider = new GenericProvider('123', 'http:executorUrl', { platform: 'win' }, {});
await provider.createDriver();
spyOn(DesktopMetaData.prototype, 'devicePixelRatio')
.and.returnValue(1);
spyOn(Driver.prototype, 'executeScript')
.and.returnValue({ value: [0, 0] });
spyOn(Driver.prototype, 'rect').and.returnValue(Promise.resolve(mockLocation));
provider.pageYShiftFactor = -10;
});
Expand Down

0 comments on commit 6c9f693

Please sign in to comment.