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

fix failing tests #165

Merged
merged 4 commits into from
Dec 12, 2020
Merged

fix failing tests #165

merged 4 commits into from
Dec 12, 2020

Conversation

jebej
Copy link
Contributor

@jebej jebej commented Dec 10, 2020

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
Copy link
Contributor Author

jebej commented Dec 10, 2020

closes #156

@hustf
Copy link
Collaborator

hustf commented Dec 12, 2020

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
Copy link
Collaborator

hustf commented Dec 12, 2020

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

@jebej
Copy link
Contributor Author

jebej commented Dec 12, 2020

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 jebej changed the title fix large data structures printing test fix failing tests Dec 12, 2020
@jebej
Copy link
Contributor Author

jebej commented Dec 12, 2020

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
Copy link
Contributor Author

jebej commented Dec 12, 2020

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

@jebej jebej force-pushed the fix-printing-test branch from c8cbfea to 88f34e6 Compare December 12, 2020 19:24
@jebej jebej force-pushed the fix-printing-test branch from 88f34e6 to 788ef2b Compare December 12, 2020 19:56
@jebej
Copy link
Contributor Author

jebej commented Dec 12, 2020

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

@hustf
Copy link
Collaborator

hustf commented Dec 12, 2020

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

@jebej
Copy link
Contributor Author

jebej commented Dec 12, 2020

0.7 passes now

@jebej jebej force-pushed the fix-printing-test branch from 788ef2b to f1d00a6 Compare December 12, 2020 20:25
@jebej
Copy link
Contributor Author

jebej commented Dec 12, 2020

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
Copy link
Contributor Author

jebej commented Dec 12, 2020

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
Copy link
Collaborator

hustf commented Dec 12, 2020

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:

  • julia_version: 0.7
  • julia_version: 1
  • julia_version: nightly

In .travis.yml:
julia:
- 0.7
- 1.0

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

@jebej
Copy link
Contributor Author

jebej commented Dec 12, 2020

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
Copy link
Contributor Author

jebej commented Dec 12, 2020

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
Copy link
Collaborator

hustf commented Dec 12, 2020

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
Copy link
Contributor Author

jebej commented Dec 12, 2020

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
Copy link
Contributor Author

jebej commented Dec 12, 2020

all good! ✅

@hustf hustf merged commit a47f2ab into JuliaWeb:master Dec 12, 2020
@jebej jebej deleted the fix-printing-test branch December 12, 2020 21:32
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.

Test fails on Julia master due to changes in printing
2 participants