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: set correct max test shards when arm devices are configured #2404

Merged
merged 1 commit into from
Jul 12, 2023
Merged

fix: set correct max test shards when arm devices are configured #2404

merged 1 commit into from
Jul 12, 2023

Conversation

adamvduke
Copy link
Contributor

@adamvduke adamvduke commented Jul 11, 2023 •
edited
Loading

Current correct behavior:

  • If max-test-shards is set to -1, AND ONLY physical devices are configured, Flank will adjust max-test-shards to the upper limit for physical devices
  • If max-test-shards is set to -1, AND ONLY virtual devices are configured, Flank will adjust max-test-shards to the upper limit for virtual devices
  • If max-test-shards is set to a value higher than the upper limit for physical devices AND ONLY physical devices are configured, Flank will throw a configuration error
  • If max-test-shards is set to a value higher than the upper limit for physical devices AND BOTH physical and virtual devices are configured, Flank will adjust max-test-shards to the upper limit for physical devices
  • If max-test-shards is set to a value higher than the upper limit for virtual devices, Flank will throw a configuration error

Current incorrect behavior:

  • If max-test-shards is set to -1, and ARM devices are configured, Flank will adjust max-test-shards to the upper limit for virtual devices, which is currently higher than the documented upper limit for ARM devices
  • If max-test-shards is set to a value higher than the upper limit for ARM devices and ARM devices are configured, Flank will adjust max-test-shards to the upper limit for virtual devices, which is currently higher than the documented upper limit for ARM devices

New behavior:

  • If max-test-shards is set to -1, and ARM devices are configured, Flank will adjust max-test-shards to the upper limit for ARM devices
  • If max-test-shards is set to a value higher than the upper limit for ARM devices AND ONLY ARM devices are configured, Flank will throw a configuration error
  • If max-test-shards is set to a value higher than the upper limit for ARM devices AND BOTH ARM and non-ARM devices are configured, Flank will adjust max-test-shards to the upper limit for ARM devices

Fixes #2401

  • Documented
  • Unit tested
  • Integration tests updated

@github-actions
Copy link
Contributor

github-actions bot commented Jul 11, 2023 •
edited
Loading

Timestamp: 2023-07-12 17:53:37
Buildscan url for ubuntu-workflow run 5534790242

tonybaroneee
tonybaroneee requested changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@tonybaroneee tonybaroneee left a comment

Choose a reason for hiding this comment

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

Nice! nit: "ARM" -> "Arm"

test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt Outdated Show resolved Hide resolved
@adamvduke adamvduke force-pushed the adamvduke/set-correct-max-test-shards-for-arm-devices branch 2 times, most recently from 651fe0b to 085ed8d Compare July 12, 2023 17:45
@adamvduke adamvduke force-pushed the adamvduke/set-correct-max-test-shards-for-arm-devices branch from 085ed8d to 384373d Compare July 12, 2023 17:47
@adamvduke
Copy link
Contributor Author

@flank-it

@github-actions
Copy link
Contributor

github-actions bot commented Jul 12, 2023 •
edited
Loading

Integration tests succeed for all OSs ✅
Windows Build scan:
MacOS Build scan:
Linux Build scan:
Workflow run https://github.com/Flank/flank/actions/runs/5534820792

@adamvduke adamvduke merged commit abbfdd7 into master Jul 12, 2023
@adamvduke adamvduke deleted the adamvduke/set-correct-max-test-shards-for-arm-devices branch July 12, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonybaroneee tonybaroneee tonybaroneee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

maximum max-test-shards incorrect for arm virtual devices
2 participants
@adamvduke @tonybaroneee