JuliaWeb / WebSockets.jl

A WebSockets library for Julia
MIT License
157 stars 58 forks source link

fix failing tests #165

Closed jebej closed 3 years ago

jebej commented 3 years ago

I'm not sure exactly when the change in Julia happened so I just set the version to above 1.0 which should be the same as 0.7. Closes #156

jebej commented 3 years ago

closes #156

hustf commented 3 years ago

Thanks, Jebej! Could you also extend atline 60: ´´´ @test Logging.default_metafmt(Error, Main, :g, :i, "", 0) == if VERSION > v"1.5.1" (:light_red, "Error:", "@ Main :0") else (:yellow, "Error:", "@ Main :0") end ´´´

hustf commented 3 years ago

Also, if you would be so kind, in file 'src/Logger/websocketlogger.jl', line 9: ´´´ logging_error, _invoked_shouldlog ´´´

jebej commented 3 years ago

Also, if you would be so kind, in file 'src/Logger/websocketlogger.jl', line 9: ´´´ logging_error, _invoked_shouldlog ´´´

I would have never figured that out!

jebej commented 3 years ago

Thanks, Jebej! Could you also extend atline 60: ´´´ @test Logging.default_metafmt(Error, Main, :g, :i, "", 0) == if VERSION > v"1.5.1" (:light_red, "Error:", "@ Main :0") else (:yellow, "Error:", "@ Main :0") end ´´´

this one seems to be failing on 0.7

jebej commented 3 years ago

the new import is causing a warning on 0.7, any idea on which version to start adding it?

jebej commented 3 years ago

okay I think that's enough here, more tests are failing on nightly but they can be fixed in an other PR

hustf commented 3 years ago

We're simply dropping 0.7 from the tests. I'll do that.

jebej commented 3 years ago

0.7 passes now

jebej commented 3 years ago

Okay, I think with that last push, all tests pass, though there is an error on nightly with

_uv_status_tuple
      @ C:\projects\websockets-jl\src\show_ws.jl:73

probably due to changes in base.

jebej commented 3 years ago

We're simply dropping 0.7 from the tests. I'll do that.

I can swap it for 1.0 since it's LTS. Also I'm not sure how to allow the nightly to fail, maybe the line that says

matrix:
  allow_failures:
  - julia_version: latest

needs to be changed to nightly instead of latest?

hustf commented 3 years ago

Appveyor hasn't completed yet.

I'm a bit rusty and don't remember the syntax for allowing failures on nightly. But a failure on nightly is not a bad thing, I suppose. It looks like something that ought to be fixed.

However, for dropping 0.7 it would be deleting the relevant lines:

In appveyor.yml: matrix:

In .travis.yml: julia:

Again, if you don't, I'll do it. But it is tidier to do everything in a pull request.

jebej commented 3 years ago

Appveyor hasn't completed yet.

You can look now, and the last one also passed, (again, except nightly)

But a failure on nightly is not a bad thing, I suppose. It looks like something that ought to be fixed.

So what is the decision, leave as is? The advantage would be to get a green CI again.

However, for dropping 0.7 it would be deleting the relevant lines:

Should I replace 0.7 by 1.0 (the current LTS)?

jebej commented 3 years ago

I replaced 0.7 with 1.0 & allowed failure on nightly, it can be changed back when the code is fixed.

CI should be green now 🤞

hustf commented 3 years ago

We'll just merge and tag when we get green checks.

This error on nightly requires somebody put the think hat on. It should be a new issue and fixed elsewhere:

  LoadError: TypeError: in typeassert, expected Ptr{Nothing}, got a value of type Ptr{UInt64}
  Stacktrace:
    [1] getproperty
      @ .\stream.jl:60 [inlined]

Then a new tag before including HTTP 0.9, I think. I don't know if there are interface changes.

jebej commented 3 years ago

Yes, it looks like this part of the code was copied from base, so I suppose it's a question of copying whatever change was made.

As soon as you merge here I'll rebase the other PR.

jebej commented 3 years ago

all good! ✅