Skip to content

Conversation

@HakkyuKim
Copy link
Contributor

@HakkyuKim HakkyuKim commented Sep 28, 2021

Sorry for the unexpected huge diff, adding default targets was not as straightforward as I originally thought. I may have done some over refactoring but I already had planned the changes for #230. It was simpler to work with the refactored code so I decided to apply it a bit early.

This PR must be merged after adding the --platforms option in integration_test.yml.

Summary:

  • Run clean command after each test to save disk space. (Nothing has changed between line 136~237)
  • Don't catch exceptions and print stack trace.
  • Fix incorrectly used nested f-string.
  • Introduce Target which encapsulates a Tizen device.
  • Introduce TargetManager which finds and manages a collection of Targets.
  • Update outdated docstrings.
  • Define some words used in code, such as target, platform, etc.
  • Allow tool to select all connected targets for test devices by default.
  • Add --platforms option which takes inputs of the form <device_profile>-<tizen_version>, the tool will find all satisfying targets and serve them as test devices.
# When connected with 4 emulators with different platforms.
./tools/run_command.py test --plugins wakelock
============= TEST RESULT =============
SUCCEEDED: wakelock ('mobile-6.0', 'M-6.0-x86', 'emulator-26131')
SUCCEEDED: wakelock ('wearable-5.5', 'W-5.5-circle-x86', 'emulator-26121')
SUCCEEDED: wakelock ('wearable-4.0', 'w-0521-1', 'emulator-26111')
SUCCEEDED: wakelock ('tv-6.0', 'T-samsung-6.0-x86', 'emulator-26101')
All tests passed!

./tools/run_command.py test --plugins wakelock --platforms wearable-5.5
============= TEST RESULT =============
SUCCEEDED: wakelock ('wearable-5.5', 'W-5.5-circle-x86', 'emulator-26121')
All tests passed!

- Don't catch exceptions.
- Fix incorrectly used nested f-string.
@github-actions github-actions bot added the tools label Sep 28, 2021
Copy link
Contributor

@bbrto21 bbrto21 left a comment

Choose a reason for hiding this comment

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

LGTM

@bbrto21
Copy link
Contributor

bbrto21 commented Sep 29, 2021

I can't test with wearable device not emulator.
because, the Tizen version includes the patch number on device.
Can someone test below command on product? I only have a test device with unofficial platform image now.

sdb capability 
platform_version:5.5.0.2

@HakkyuKim
Copy link
Contributor Author

@bbrto21
I did not realize there were more numbers behind the first dot, I'll fix this.

@HakkyuKim
Copy link
Contributor Author

@bbrto21 I did not realize there were more numbers behind the first dot, I'll fix this.

Actually, I've set the default target as wearable-5.5 for now and since wearable-5.5.0.2 is technically not the exact version, the error message I've seen offline on your machine is correct.

I thought about improving this, but left it as a TODO.

if not test_targets:
# (TODO: HakkyuKim) Improve logic for setting default targets.
test_targets.append(_DEFAULT_TEST_TARGET)

Since the issue came up, I'll find a better way to set default targets.

@HakkyuKim HakkyuKim marked this pull request as draft September 29, 2021 04:50
- Update outdated docstring.
- Add target definition in docstring.
- Update variable names based on definition.
By default, the tools selects all connected targets
for testing.
- Add `--platforms` option to specify group of testing
targets.
@HakkyuKim HakkyuKim changed the title Run clean command after each test Refactor, run clean command after each test, improve logic for setting default targets. Sep 30, 2021
@HakkyuKim HakkyuKim marked this pull request as ready for review September 30, 2021 05:02
@HakkyuKim
Copy link
Contributor Author

@bbrto21
You should be able to test with any connected targets now.

@HakkyuKim HakkyuKim requested a review from bbrto21 September 30, 2021 05:04
@HakkyuKim HakkyuKim added the no merge This PR is not ready for merge yet label Sep 30, 2021
@bbrto21
Copy link
Contributor

bbrto21 commented Oct 1, 2021

@HakkyuKim
Thank you, it works fine :)

@HakkyuKim HakkyuKim removed the no merge This PR is not ready for merge yet label Oct 5, 2021
@HakkyuKim HakkyuKim merged commit a96baa4 into flutter-tizen:master Oct 5, 2021
@HakkyuKim HakkyuKim deleted the tools-fix branch December 21, 2021 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants