Fail early when XamariniOS builds are run on hosted windows or linux #7184
Conversation
madhurig
force-pushed
the
users/madhurig/xamarinios
branch
from
73e1e35 to
1072c08
Compare
madhurig
force-pushed
the
users/madhurig/xamarinios
branch
from
1072c08 to
4f8330e
Compare
madhurig
changed the title
madhurig
changed the title
Tasks/XamariniOSV2/xamarinios.ts
Outdated
| @@ -28,6 +29,11 @@ async function run() { | |||
| try { | |||
| tl.setResourcePath(path.join(__dirname, 'task.json')); | |||
|
|
|||
| // Check platform is macOS since demands are not evaluated on Hosted pools | |||
| if (!/^darwin/.test(os.platform())) { | |||
| throw tl.loc('BuildRequiresMac'); | |||
There was a problem hiding this comment.
An improvement is throw new Error(tl.loc('BuildRequiresMac')); It will preserve the stack trace correctly.
madhurig
force-pushed
the
users/madhurig/xamarinios
branch
from
433b53d to
4fc2282
Compare
|
Builds passed on re-run, so merging. |
| @@ -14,6 +14,24 @@ describe('XamariniOS L0 Suite', function () { | |||
|
|
|||
| }); | |||
|
|
|||
| it('run XamariniOSV2 with all default inputs', (done) => { | |||
There was a problem hiding this comment.
This needs to be a function for this.timeout(1000) to do anything.
There was a problem hiding this comment.
Good point, I recollect now that timeouts don't apply otherwise. Will fix in my next PR.
|
|
||
| assert(tr.ran('/home/bin/nuget restore src/project.sln'), 'it should have run nuget restore'); | ||
| assert(tr.ran('/home/bin/msbuild src/project.sln /p:Configuration=Release /p:Platform=iPhone'), 'it should have run msbuild'); | ||
| assert(tr.invokedToolCount == 3, 'should have only run 3 commands'); |
There was a problem hiding this comment.
Done, these were legacy tests I moved.
| assert(tr.stderr.length == 0, 'should not have written to stderr'); | ||
| assert(tr.succeeded, 'task should have succeeded'); | ||
|
|
||
| done(); |
There was a problem hiding this comment.
You don't need to call done unless you are running async code.
There was a problem hiding this comment.
Inside the function, done() needs to be called.
| tr.setInput('provProfileUuid', ''); | ||
|
|
||
| // provide answers for task mock | ||
| let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{ |
There was a problem hiding this comment.
I made a change to TaskLibAnswers so we shouldn't have to type assert anymore. Removing the type assertion will catch mocks that don't work (getVariable in this case).
There was a problem hiding this comment.
This task in on older version of the lib!
| @@ -46,5 +47,10 @@ let a: ma.TaskLibAnswers = <ma.TaskLibAnswers>{ | |||
| }; | |||
| tr.setAnswers(a); | |||
|
|
|||
| os.platform = () => { | |||
| } | ||
| } | ||
|
|
||
| async function run() { | ||
| try { | ||
| tl.setResourcePath(path.join(__dirname, 'task.json')); | ||
|
|
||
| // Check platform is macOS since demands are not evaluated on Hosted pools | ||
| if (!/^darwin/.test(os.platform())) { |
There was a problem hiding this comment.
Why use a regex? The list of possible platforms is documented, so you can just check === 'darwin'
There was a problem hiding this comment.
Changing to !== 'darwin'
Since demands are not validated in Hosted pools now, users can run iOS builds on windows and linux hosted pools. Our tasks don't handle that since they were written assuming demand validation would be done.
I started modifying the Xamarin iOS task but Xcode, InstallAppleCertificate, InstallAppleProvisioningProfile will also need to be updated to complete this experience.
The legacy tests are not fixed, so they will fail on windows and linux runs.