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.

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

Entry point on github actions #334

Merged
merged 12 commits into from
Jun 2, 2020
Merged

Entry point on github actions #334

merged 12 commits into from
Jun 2, 2020

Conversation

alorma
Copy link
Member

@alorma alorma commented Dec 2, 2019

From #333

Add Github actions CI to run instrumentation tests on every push on master and PR

@@ -46,18 +47,21 @@ public void iconMenuClick_byTitle() {
}

@Test
@Ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

Those are flaky tests on CI, we must try to check why...

Copy link
Member

Choose a reason for hiding this comment

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

I think they pass in the local machine...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's why I marked them as flaky, maybe we can merge this PR and try to fix the tests?

Copy link
Member

Choose a reason for hiding this comment

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

You marked them as @Ignore, not flaky.

Perwaps we could add a message that explains that it doesn't pass on CI but it does in the local machine? Just to help to ourselves of the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's see if @FlakyTest works

Copy link
Member

Choose a reason for hiding this comment

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

Why do they fail in CI? Do we have any idea?
Maybe we could try running it locally with the exact same emulator as CI, sometimes that gives it away 🤷‍♂

@alorma alorma removed the wip label Dec 2, 2019
@alorma alorma requested review from a team and rocboronat December 2, 2019 10:35
Copy link
Member

@rocboronat rocboronat left a comment

Choose a reason for hiding this comment

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

The tests are failing right now

@alorma
Copy link
Member Author

alorma commented May 28, 2020

I've updated the emulator plugin version to 2.

It allows to create a matrix of emulators, cool!

There are still failing tests...

I think...

Could we merge this, and let's fix tests later? This way at least we can track failing tests, as it happened on #350 , we didn't know that some tests were failing

@rocboronat
Copy link
Member

But we're going to have tests that pass on local but not in the CI... so we'll ignore them I think 😅

@alorma
Copy link
Member Author

alorma commented May 28, 2020

Not really.

Isn't it better to have CI with failing tests that no CI with no failing tests?

Right now on travis we are not checking that...

@alorma
Copy link
Member Author

alorma commented May 28, 2020

I reproduced the failing tests on local.

@rocboronat
Copy link
Member

Isn't it better to have CI with failing tests that no CI with no failing tests?

Hum, that's ok if you had green builds before, but if you never had the suite passing, I don't see the value but having red builds on production, which is eye-painful hehe... but it's just a personal decision I think. I understand the CI as a fast way to see errors before sending things to production. If it says that everything is incorrect but actually all is working as expected, I don't know...

@Sloy
Copy link
Member

Sloy commented May 28, 2020

I agree with @rocboronat. Having a red build all the time doesn't help. You get used to it and ignore it :(

@rocboronat
Copy link
Member

@alorma is very near to achieve having the tests passing here. Can't wait to see them working! ❤️

@alorma
Copy link
Member Author

alorma commented May 29, 2020

Look @Sloy @rocboronat it's green!

@rocboronat
Copy link
Member

Cool! If you think it's stable enough, feel free to merge. If it doesn't behave as we expect, we always have time to disable it. Let's try it in the real world!

Thanks for working on it, Bernat! 😊 For sure a lot of people will be "influenced" by this PR 😄

@alorma alorma merged commit 9a9aa3c into master Jun 2, 2020
@alorma alorma deleted the actions_workflow branch June 2, 2020 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants