-
Notifications
You must be signed in to change notification settings - Fork 167
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
AWS Device Farm #2831
AWS Device Farm #2831
Conversation
47a79f1
to
b27362d
Compare
Pull Request Test Coverage Report for Build 2061381366
💛 - Coveralls |
b27362d
to
ab05f32
Compare
0d138fc
to
dc14fd2
Compare
dc14fd2
to
c3f7dd0
Compare
# Conflicts: # .github/templates/main.yml # .github/workflows/main.yml
cabdbdc
to
00bb2f8
Compare
This reverts commit bfc9d81.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, it seems fine to me, though I'd prefer to move a lot of the complexity to the device farm action as it seems counterproductive to maintain it across multiple repositories .
env: | ||
REALM_DISABLE_ANALYTICS: true | ||
DOTNET_NOLOGO: true | ||
jobs: | ||
run-tests: | ||
runs-on: macos-latest | ||
runs-on: windows-latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this run on Linux instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't build Xamarin.Android projects on Linux.
if: steps.avd-cache.outputs.cache-hit != 'true' | ||
uses: reactivecircus/android-emulator-runner@v2 | ||
run: msbuild Tests/Tests.Android -t:SignAndroidPackage -p:Configuration=Release -restore -p:RestoreConfigFile=Tests/Test.NuGet.Config -p:UseRealmNupkgsWithVersion=${{ inputs.version }} -p:AndroidSupportedAbis=armeabi-v7a -p:AndroidUseSharedRuntime=False -p:EmbedAssembliesIntoApk=True | ||
- name: Configure AWS Credentials |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to configure AWS credentials here? Shouldn't this be done by the device farm action?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a convention with actions using the AWS SDK. Besides, in this specific case I need to use some AWS PowerShell cmdlets and they also need the same credentials.
app_type: ANDROID_APP | ||
test_type: APPIUM_PYTHON | ||
test_package_file: https://github.com/realm/aws-devicefarm-sample-data/releases/download/0.3/sample_env_python3.zip | ||
test_package_type: APPIUM_PYTHON_TEST_PACKAGE | ||
test_spec_file: test_spec.yaml | ||
test_spec_type: APPIUM_PYTHON_TEST_SPEC | ||
remote_src: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These don't seem like something that should be configurable. It's fine to leave them like this for now, but I'd rather set them as defaults in the action and not have us specify them every time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll wait until I have a working version for iOS as well, or until we need to integrate this into other SDKs. It's not unlikely that other SDKs will use a different test package or spec. But I agree, this can use a refactoring on a second pass.
- run: | | ||
Expand-Archive 'Customer Artifacts.zip' -DestinationPath artifacts | ||
Import-Module AWSPowerShell | ||
$jobs = Get-DFJobList -Arn ${{ steps.run_tests.outputs.arn }} | ||
$suites = Get-DFSuiteList -Arn $jobs[0].Arn | ||
$artifacts = Get-DFArtifactList -Arn $suites[1].Arn -Type File | Where-Object { $_.Name -EQ "Logcat" } | ||
echo "::group::Logcat" | ||
Invoke-WebRequest -Uri $artifacts[0].Url | Select-Object -Expand RawContent | ||
echo "::endgroup::" | ||
name: Fetch test artifacts | ||
- run: | | ||
echo "::group::Data" | ||
echo (ConvertFrom-Json '${{ steps.run_tests.outputs.data }}' | ConvertTo-Json) | ||
echo "::endgroup::" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This too seems like something that should be done internally in the device farm action rather than here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I filed realm/aws-devicefarm#6 to track this. It's something @bwachter and I already discussed.
Description
Run Android tests on AWS Device Farm physical devices.