Skip to content
This repository has been archived by the owner on Jan 2, 2021. It is now read-only.

Disable the 8.8 Windows tests, too unreliable #850

Merged
merged 2 commits into from
Oct 6, 2020

Conversation

pepeiborra
Copy link
Collaborator

No description provided.

@pepeiborra pepeiborra requested a review from jneira October 5, 2020 19:12
Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

I'll try to investigate what is going on with failing reruns

@jneira
Copy link
Member

jneira commented Oct 5, 2020

Mmm could we at least compile the project, including compile test if possible?

@jneira jneira self-requested a review October 5, 2020 20:57
@pepeiborra
Copy link
Collaborator Author

Mmm could we at least compile the project, including compile test if possible?

Why? If it compiles with 8.6 Win and 8.10 Linux, doesn't that imply 8.10 Win?

@jneira
Copy link
Member

jneira commented Oct 5, 2020

Why? If it compiles with 8.6 Win and 8.10 Linux, doesn't that imply 8.10 Win?

Mmm, i am afraid there is enough cpp conditions over ghc version and windows (in ghcide and upstream packages) to break that for some (enough?) cases

@jneira
Copy link
Member

jneira commented Oct 6, 2020

i've checked that, only if you set explicitly the rerun log with -rerun-log-file , -rerun works locally so maybe we could restore tests for all versions.
Do we have some evidence that -rerun works in linux? i'll take a look

@pepeiborra
Copy link
Collaborator Author

It works fine in Linux

@pepeiborra
Copy link
Collaborator Author

My motivation here is to keep CI reliable and fast. I had the pleasure of merging ~30 PRs with an unreliable CI not so long ago: it was so painful that I invested some time in improving the CI pipelines with your awesome help. That effort involved disabling the Windows jobs which were way too unreliable. When it comes to Windows, I don't even have a Windows machine to reproduce and diagnose issues.

Recently you did a lot of awesome work to reenable the Windows jobs. I just think that they are not ready and a bit more work is needed.

Windows jobs should not make CI materially slower nor unreliable. The current metrics are:

image

With the goal of preserving the job times under 30m and reliability at 90%, I propose the following:

  1. Let's merge this PR to disable 8.8 and 8.10 Windows jobs
  2. Someone send a PR that reenables the Windows jobs as they see fit.
  3. Run that PR 10 times
  4. When it passes 9 times and runs under 30m, it's ready to merge.

@wz1000
Copy link
Collaborator

wz1000 commented Oct 6, 2020

I agree with this.

But sometimes it is good to have Windows jobs to catch obvious errors. Can we run the windows jobs, but don't require them to pass before we can merge?

@pepeiborra
Copy link
Collaborator Author

I agree with this.

But sometimes it is good to have Windows jobs to catch obvious errors. Can we run the windows jobs, but don't require them to pass before we can merge?

That should hopefully fit the requirements of 30m and 90% reliability, so I have nothing against it if you or anyone else wants to submit a follow-up PR.

But to be clear, I do want to run and pass the Windows jobs. What I am asking is that a) someone else takes the burden of maintaining them and b) the unreliable tests are conditionally disabled in Windows. I don't think that running the Windows jobs in ignore mode adds the same value.

@jneira
Copy link
Member

jneira commented Oct 6, 2020

I've just checked if rerun is working in linux for ghcide and i am afraid that it is not, see: https://dev.azure.com/jneira/haskell-language-server/_build/results?buildId=865&view=logs&j=4c83789f-ef27-517f-6ab9-6fdd4fc2b12a&t=18529ee9-de7e-5f9c-736e-215e246b6053
EDITED: i've double checked the linux job and the rerun only is enabled for second and third, so the second pass is always with all tests, if i am understand it correctly
I am preparing a pr to make it work for both os'swindows and see if that way it is reliable enough

Recently you did a lot of awesome work to reenable the Windows jobs. I just think that they are not ready and a bit more work is needed.

Fair enough, but before that change the project was being compiled, including tests. So i would prefer to let it as it was for 8.8 and 8.10, keeping them as required.

@wz1000
Copy link
Collaborator

wz1000 commented Oct 6, 2020

I don't think that running the Windows jobs in ignore mode adds the same value.

Yes, but as someone who has no access to a windows machine, it is useful to be able to take a quick peek at CI to see that nothing is seriously broken, even if some tests are failing due to the usual unreliability.

@pepeiborra
Copy link
Collaborator Author

I've just checked if rerun is working in linux for ghcide and i am afraid that it is not, see: https://dev.azure.com/jneira/haskell-language-server/_build/results?buildId=865&view=logs&j=4c83789f-ef27-517f-6ab9-6fdd4fc2b12a&t=18529ee9-de7e-5f9c-736e-215e246b6053
EDITED: i've double checked the linux job and the rerun only is enabled for second and third, so the second pass is always with all tests, if i am understand it correctly
I am preparing a pr to make it work for both os'swindows and see if that way it is reliable enough

No, your understanding is not correct. The first pass does not need --rerun

Recently you did a lot of awesome work to reenable the Windows jobs. I just think that they are not ready and a bit more work is needed.

Fair enough, but before that change the project was being compiled, including tests. So i would prefer to let it as it was for 8.8 and 8.10, keeping them as required.

No, I disabled the Windows jobs completely. You reenabled them here: #808

@pepeiborra
Copy link
Collaborator Author

I don't think that running the Windows jobs in ignore mode adds the same value.

Yes, but as someone who has no access to a windows machine, it is useful to be able to take a quick peek at CI to see that nothing is seriously broken, even if some tests are failing due to the usual unreliability.

Yes, I understand that. I am in the same position. I'm just saying that that's not the ideal outcome

@jneira
Copy link
Member

jneira commented Oct 6, 2020

No, I disabled the Windows jobs completely. You reenabled them here: #808

Ok, but they were enabled before #794 and the reason to disable the compiling at that point (not working azure cache) is fixed, right?

But well, i'll try to reenable them with tests fullfilling those conditions, they are totally reasonable.

@pepeiborra pepeiborra merged commit d874a32 into haskell:master Oct 6, 2020
@jneira
Copy link
Member

jneira commented Oct 6, 2020

No, your understanding is not correct. The first pass does not need --rerun

@pepeiborra i am checking tasty behaviour in windows and if i dont set --rerun tasty does not create .test-rerun-log so the next run can't use it to filter failing tests. Otoh --rerun executes all tests if the previous run is all green by default so it seems it is needed in the first step, i we want make the second run handling only failing ones.
maybe the behaviour is different in linux? or did you want to run the full suite at least two times?

@pepeiborra
Copy link
Collaborator Author

pepeiborra commented Oct 6, 2020

I was wrong then, thanks for checking. No need to run the full suite two times!

pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Disable the 8.8 Windows tests, too unreliable

* Disable the 8.10 Windows tests, idem
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Disable the 8.8 Windows tests, too unreliable

* Disable the 8.10 Windows tests, idem
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Disable the 8.8 Windows tests, too unreliable

* Disable the 8.10 Windows tests, idem
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants