Skip to content

Add integration test workflow #169

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
Merged
HakkyuKim merged 8 commits into from
Oct 5, 2021
Merged

Add integration test workflow #169

HakkyuKim merged 8 commits into from
Oct 5, 2021

Conversation

@HakkyuKim
Copy link
Contributor

@HakkyuKim HakkyuKim commented Aug 6, 2021 •
edited
Loading

Github workflows script that runs integration test on pull requests. This PR should be merged after setting up self-hosted runner.

Notes:

  • Script doesn't run on forked repositories to silence no self-hosted runner error.
  • Currently only a single self-hosted runner runs on the server machine, this may lag CI when multiple commits are submitted to the repo in a short time period. I have a few solutions in mind but need to verify some things first. Anyhow, it will be implemented in the next PR.
  • Currently only supports testing on wearable-5.5 emulator.

Testing done on #228.

@HakkyuKim HakkyuKim mentioned this pull request Aug 6, 2021
Closed
4 tasks
@bbrto21
Copy link
Contributor

bbrto21 commented Aug 18, 2021

please, rebase first

@HakkyuKim HakkyuKim marked this pull request as ready for review August 18, 2021 08:32
@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Aug 18, 2021

We can now merge this after adding a self-hosted runner for this repository.

bwikbs
bwikbs approved these changes Aug 18, 2021
View reviewed changes
Copy link
Member

@bwikbs bwikbs left a comment

Choose a reason for hiding this comment

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

Oh! This is what I'm looking forward to personally! 🔥

@bbrto21
Copy link
Contributor

bbrto21 commented Aug 19, 2021

self-hosted runner for this repository.

@HakkyuKim Is our self-hosted runner ready?

@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Aug 20, 2021

@bbrto21 I'm working on something else in our internal git at the moment, I'll bring it up next Monday!

@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Sep 1, 2021

I'm really sorry about the delay, setting up a server inside the company is a slow process...

@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Sep 10, 2021 •
edited
Loading

To share the process so far...I'm requesting one security approval after another. Apparently some urls are blocked by firewalls even with extern network, unfortunately GitHub is one of them.

@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Sep 13, 2021

The test server can now listen to Github workflows from an external network, https://github.com/HakkyuKim/plugins/pull/14/checks?check_run_id=3585756046. However, wearable emulators just randomly crash even when no app is running. Mobile and TV emulators can't even get booted. I'm planning to reinstall the host OS, file an issue to Tizen SDK team, or use another server machine.

@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Sep 23, 2021

Reinstalling the OS did not work, I've filed an issue to Tizen SDK team.

@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Sep 24, 2021

The test server finally works, @swift-kim could you grant me the permission to add self-hosted runner for this repo so I can test this out?

@HakkyuKim HakkyuKim added the no merge This PR is not ready for merge yet label Sep 24, 2021
swift-kim
swift-kim reviewed Sep 27, 2021
View reviewed changes
@HakkyuKim HakkyuKim marked this pull request as draft September 27, 2021 01:49
@HakkyuKim HakkyuKim removed the no merge This PR is not ready for merge yet label Sep 28, 2021
@HakkyuKim HakkyuKim marked this pull request as ready for review September 28, 2021 05:46
WonyoungChoi
WonyoungChoi reviewed Sep 28, 2021
View reviewed changes
WonyoungChoi
WonyoungChoi reviewed Sep 28, 2021
View reviewed changes
WonyoungChoi
WonyoungChoi reviewed Sep 28, 2021
View reviewed changes
WonyoungChoi
WonyoungChoi reviewed Sep 28, 2021
View reviewed changes
WonyoungChoi
WonyoungChoi reviewed Sep 28, 2021
View reviewed changes
@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Sep 30, 2021 •
edited
Loading

Should workflow be merged after setting multiple self-hosted runners with different device profiles? If this PR is merged, only a single self-hosted runner will be used for CI, testing plugin integration test on wearable-5.5 emulator.

#229 and #232 are required to run multiple self-hosted runners with different device profiles. running on tv emulators needs extra implementation

bbrto21
bbrto21 approved these changes Oct 1, 2021
View reviewed changes
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
I think it's meaningful even if it test plugins only on wearable emulator.
and if we use this, we may find new issues that we didn't expect.

@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Oct 5, 2021

Let's merge #229 first so we can add the --platforms options in this PR.

@HakkyuKim HakkyuKim added the no merge This PR is not ready for merge yet label Oct 5, 2021
@HakkyuKim
Update workflow script
cbf7e20
@HakkyuKim
Use latest flutter-tizen
aaed7db 
- Split long command to multiple lines.
- Remove commented code.
@HakkyuKim
Exclude tizen_app_control from test
d6ac2fe
HakkyuKim and others added 5 commits October 5, 2021 09:33
@HakkyuKim
Clarify a name in workflow step
154afcc
@HakkyuKim
Remove unnessary condition check
d142039
@HakkyuKim
Use checkout actions for flutter-tizen install
5b66a05 
- Remove setting legacy variable.
- Remove `pwd` and use dot syntax when referencing
  from repo root.
- export path variable with custom path first.
@HakkyuKim
Fix incorrect package name
1e75e08 
wifi_info -> wifi_info_flutter
@HakkyuKim
Use --platforms option
15dda0c
@HakkyuKim
Copy link
Contributor Author

HakkyuKim commented Oct 5, 2021

Rebased with current master to use the --platforms option.
The only thing changed since the latest review is:

  • Use the --platforms option in workflows script: 15dda0c
  • Fix misused package name: 1e75e08

@HakkyuKim HakkyuKim removed the no merge This PR is not ready for merge yet label Oct 5, 2021
@HakkyuKim HakkyuKim merged commit 085cdc4 into flutter-tizen:master Oct 5, 2021
@HakkyuKim HakkyuKim deleted the add-integration-test-workflow branch December 21, 2021 04:34
bwikbs pushed a commit to bwikbs/plugins that referenced this pull request Mar 26, 2022
@HakkyuKim @bwikbs
Add integration test workflow (flutter-tizen#169)
537c0ce 
* Update workflow script

* Use latest flutter-tizen

- Split long command to multiple lines.
- Remove commented code.

* Exclude tizen_app_control from test

* Clarify a name in workflow step

* Remove unnessary condition check

* Use checkout actions for flutter-tizen install

- Remove setting legacy variable.
- Remove `pwd` and use dot syntax when referencing
  from repo root.
- export path variable with custom path first.

* Fix incorrect package name

wifi_info -> wifi_info_flutter

* Use --platforms option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@swift-kim swift-kim swift-kim left review comments

@bbrto21 bbrto21 bbrto21 approved these changes

@bwikbs bwikbs bwikbs approved these changes

@WonyoungChoi WonyoungChoi Awaiting requested review from WonyoungChoi

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@HakkyuKim @bbrto21 @WonyoungChoi @bwikbs @swift-kim