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.

Sign up for GitHub

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

Jump to bottom

fix(ios): regression in incremental device builds (8_1_X) #11024

Merged
merged 6 commits into from Jul 12, 2019
Merged

fix(ios): regression in incremental device builds (8_1_X) #11024

merged 6 commits into from Jul 12, 2019

Conversation

janvennemann
Copy link
Contributor

@janvennemann janvennemann added this to the 8.1.0 milestone Jul 3, 2019
@build
Copy link
Contributor

build commented Jul 3, 2019 •
edited

Fails
🚫 Tests have failed, see below for more information.
Warnings
⚠️

cli/lib/tasks/process-js-task.js#L157 - cli/lib/tasks/process-js-task.js line 157 – Avoid calling back inside of a promise. (promise/no-callback-in-promise)
⚠️

cli/lib/tasks/process-js-task.js#L164 - cli/lib/tasks/process-js-task.js line 164 – Avoid calling back inside of a promise. (promise/no-callback-in-promise)
Messages
📖

💾 Here's the generated SDK zipfile.
📖 ❌ 4 tests have failed There are 4 tests failing and 464 skipped out of 3619 total tests.

Tests:

ClassnameNameTimeError
android.emulator.Titanium.Geolocation#reverseGeocoder()0.128
Error: expected false to equal true
at Assertion.fail (/node_modules/should/cjs/should.js:275:17)
      at Assertion.value (/node_modules/should/cjs/should.js:356:19)
      at Geolocation.<anonymous> (/ti.geolocation.test.js:277:33)
android.emulator.Titanium.Geolocation#forwardGeocoder()0.044
Error: expected false to equal true
at Assertion.fail (/node_modules/should/cjs/should.js:275:17)
      at Assertion.value (/node_modules/should/cjs/should.js:356:19)
      at Geolocation.<anonymous> (/ti.geolocation.test.js:257:33)
ios.Titanium.Geolocation#reverseGeocoder()0.118
Error: expected false to equal true
fail@file:///Users/build/Library/Developer/CoreSimulator/Devices/1C556AA3-15EF-4456-B9CF-AB4E919D51D6/data/Containers/Bundle/Application/48B26653-7AC7-46E1-AA2E-9B59E38C74CE/mocha.app/node_modules/should/cjs/should.js:275:23
value@file:///Users/build/Library/Developer/CoreSimulator/Devices/1C556AA3-15EF-4456-B9CF-AB4E919D51D6/data/Containers/Bundle/Application/48B26653-7AC7-46E1-AA2E-9B59E38C74CE/mocha.app/node_modules/should/cjs/should.js:356:23
file:///Users/build/Library/Developer/CoreSimulator/Devices/1C556AA3-15EF-4456-B9CF-AB4E919D51D6/data/Containers/Bundle/Application/48B26653-7AC7-46E1-AA2E-9B59E38C74CE/mocha.app/ti.geolocation.test.js:277:36
ios.Titanium.Geolocation#forwardGeocoder()0.121
Error: expected false to equal true
fail@file:///Users/build/Library/Developer/CoreSimulator/Devices/1C556AA3-15EF-4456-B9CF-AB4E919D51D6/data/Containers/Bundle/Application/48B26653-7AC7-46E1-AA2E-9B59E38C74CE/mocha.app/node_modules/should/cjs/should.js:275:23
value@file:///Users/build/Library/Developer/CoreSimulator/Devices/1C556AA3-15EF-4456-B9CF-AB4E919D51D6/data/Containers/Bundle/Application/48B26653-7AC7-46E1-AA2E-9B59E38C74CE/mocha.app/node_modules/should/cjs/should.js:356:23
file:///Users/build/Library/Developer/CoreSimulator/Devices/1C556AA3-15EF-4456-B9CF-AB4E919D51D6/data/Containers/Bundle/Application/48B26653-7AC7-46E1-AA2E-9B59E38C74CE/mocha.app/ti.geolocation.test.js:257:36

Generated by 🚫 dangerJS against 94154af

sgtcoolguy
sgtcoolguy approved these changes Jul 3, 2019
View reviewed changes
Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

LGTM, though did not do FR.

cli/lib/tasks/process-js-task.js
@@ -110,7 +113,11 @@ class ProcessJsTask extends IncrementalFileTask {
return this.doFullTaskRun();
}

Object.keys(this.data.jsFiles).forEach(relPath => this.builder.unmarkBuildDirFile(this.data.jsFiles[relPath].dest));
this.jsFiles = this.data.jsFiles;
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so looking at the code, it appears we already compare hashes inside the hooks that processJsFile does, so we effectively skip the work on them anyways?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it will still skip unchanged files. It just goes through the processing flow to make sure the hooks are called and that the builder gets populated with required info about encrypted files.

@ssekhri
Copy link

ssekhri commented Jul 9, 2019

FR Passed. Incremental builds working fine for iOS.

@sgtcoolguy sgtcoolguy merged commit d7c4622 into tidev:8_1_X Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgtcoolguy sgtcoolguy sgtcoolguy approved these changes

Assignees
No one assigned
Labels
bug in-qe-testing 🕵 ios
Projects
None yet
Milestone
8.1.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@janvennemann @build @ssekhri @sgtcoolguy @lokeshchdhry