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

remove deprecated stuff in unittest module #17156

Merged
merged 6 commits into from
Feb 24, 2021
Merged

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Feb 23, 2021

  • use runnableExamples
  • remove one callsite
  • add two regression tests

@ringabout ringabout marked this pull request as draft February 23, 2021 15:46
@timotheecour
Copy link
Member

timotheecour commented Feb 24, 2021

after removing callsite, CI fails in in case: https://github.com/LemonBoy/jstin with nim r tests/test1.nim

after reduction, here's a repro:

import unittest
type MyFoo = object
var obj = MyFoo()
let check = 1
check(obj == obj)
/Users/timothee/git_clone/nim/temp/jstin/tests/test1.nim(16, 6) template/generic instantiation from here
/Users/timothee/git_clone/nim/temp/jstin/tests/test1.nim(19, 6) template/generic instantiation from here
/Users/timothee/git_clone/nim/Nim_prs/lib/pure/unittest.nim(706, 9) template/generic instantiation from here
/Users/timothee/git_clone/nim/temp/jstin/tests/test1.nim(16, 6) template/generic instantiation from here
/Users/timothee/git_clone/nim/temp/jstin/tests/test1.nim(19, 6) template/generic instantiation from here
/Users/timothee/git_clone/nim/Nim_prs/lib/pure/unittest.nim(706, 9) template/generic instantiation from here
/Users/timothee/git_clone/nim/Nim_prs/lib/pure/unittest.nim(691, 46) Error: internal error: expr(nkIdent); unknown node kind
              result.printOuts.add getAst(print(argStr, arg))
                                               ^
/Users/timothee/git_clone/nim/Nim_devel/compiler/ccgexprs.nim(2913, 22) compiler msg initiated here [MsgOrigin]

and here's a different issue with a similar test case:

when true:
  import unittest
  block:
    let check = 123
    var a = 1
    var b = 1
    check(a == b)
/Users/timothee/git_clone/nim/Nim_devel/lib/system/fatal.nim(53) sysFatal
Error: unhandled exception: /Users/timothee/git_clone/nim/Nim_devel/compiler/ccgexprs.nim(616, 9) `e[1].typ != nil`  [AssertionDefect]

maybe those are fixable.

Another aspect is whether we actually should deprecate callsite or not; I've argued in #11864 (comment) that maybe we shouldn't.

regardless, if we can avoid callsite here, it might be cleaner.

note

can you add those 2 minimized examples to tunittest to avoid future regressions?

@ringabout ringabout marked this pull request as ready for review February 24, 2021 02:17
@Araq Araq merged commit 99633d7 into nim-lang:devel Feb 24, 2021
ringabout added a commit to ringabout/Nim that referenced this pull request Mar 22, 2021
* remove unnecessary when statement

* remove outdated codes

* remove deprecated stuff in testament

* fix
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
* remove unnecessary when statement

* remove outdated codes

* remove deprecated stuff in testament

* fix
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.

3 participants