Add test file #5

janEbert · 2019-07-29T08:40:06Z

Correspondingly, add the generated "tags" file to .gitignore.
I tried to add "actual code" and have too many tests rather than only
complex ones.
Surely, some cases are not covered. For example, using multiple macros
for a definition.

oxinabox · 2019-07-29T15:05:03Z

Do you think we can construct some actual tests that run on CI?
They would need to be written in Bash, I guess.
Or in Julia using run(`ctag ...`)
which could be fine

janEbert · 2019-07-29T15:12:13Z

Sure, I'd love to look into that! Though I have never set up CI.
The tests would fail though as with the simple regexes, edge cases are simply not covered:

#=
const old_zero = 0
=#

... would be parsed as an identifier.

That is also why I think a proper parser would be more useful. I have thought about writing one for Universal Ctags that would be more sophisticated than the current one but it would take some time. Guess the tests failing would be the exact thing we want until then. :)

oxinabox · 2019-07-29T15:38:01Z

I think easiest would be to setup CI for julia and write the tests in julia,
after doing readlines on the genated file.

Then you could do @test_broken for the edge cases that fail.

You can copy the .travisci from any julia project to do that.

oxinabox · 2019-07-29T15:42:51Z

If you write the tests, so you can run them locally
I will make it work with Travis CI

janEbert · 2019-07-29T16:00:08Z

How would you go about using the external Universal Ctags? They don't have release tags and the binary name may be hard to find due to name overlaps with Exuberant Ctags or Etags.

Or should I just write, try and see using ctags as the executable?

If you write the tests, I will make it work with Travis CI

Thanks, but I'd actually be glad to learn by doing!

oxinabox · 2019-07-29T16:25:52Z

Can't we just use Exuberant Ctags ?
Or are we using new features? (if so that needs to go in the readme)

janEbert · 2019-07-29T20:29:58Z

You are absolutely right and I see why naming is no problem.
We do not use new features, everything is still vanilla. :)

I added the tests and Travis CI, hope you are content with the code.
I tried to make local testing easy as well by trying different binaries.

Broken features are marked depending on the identifier name or the "kind" (function, variable, struct, ...).

The tests will fail now because I based them on the updated ctags file (#4).

Project.toml

janEbert · 2019-07-30T09:01:00Z

Hey, the build keeps being stuck on "Resolving package versions..." when testing. I couldn't find any other reports via searching and have checked out other Project.tomls and runtest.jls, however, nothing seems different.
Do you have an idea what could be wrong?

test/runtests.jl

oxinabox · 2019-07-30T20:17:19Z

Do your tests work fine locally?

janEbert · 2019-07-30T20:37:52Z

Yes, I tested just now with:

$ JULIA_LOAD_PATH=$PWD julia
julia> ]
(julia-ctags) pkg> test

And it worked fine. Though as I mentioned, if the tests ran in Travis, they would fail because I based them on the new ctags file in #4.

Edit:
I also just tested using the same command Travis uses (plus the --project flag) and it did not hang up.

janEbert · 2019-07-30T21:40:21Z

Sorry to spam; after trying a lot, I still haven't figured out how to remove those duplicate commits from master. I will fix it later, though.

karlwessel · 2019-10-16T08:30:38Z

Hey, the build keeps being stuck on "Resolving package versions..." when testing. I couldn't find any other reports via searching and have checked out other Project.tomls and runtest.jls, however, nothing seems different.
Do you have an idea what could be wrong?

I tried your PR at my own machine and it gets stuck in an endless loop because none of the predefined ctags executables worked. The same probably happens to Travis.

You probably should make sure that trygeneratetags and the tests exit gracefully if no ctags executable is found and maybe add ctags to the list of ctags executables in ctagsbins.

test/test_tags.jl

m-wells · 2019-10-17T12:52:06Z

I'm not sure how much packaging you want to do with this but you could always provide a ctags build if one wasn't found either with BinaryProvider.jl or with the Anaconda Ctags package using Conda.jl.

If you go this route you would probably want to make this into a proper julia package.

oxinabox · 2019-10-17T17:01:07Z

If you go this route you would probably want to make this into a proper julia package.

That wouldn't really make sense.
This file needs to go into your ctags config

m-wells · 2019-10-17T18:01:14Z

If you go this route you would probably want to make this into a proper julia package.

That wouldn't really make sense.
This file needs to go into your ctags config

Not really. It can be passed in as an option to the ctags command via --options=pathname

oxinabox · 2019-10-17T21:19:43Z

When you do Pkg.add or even use Artifacts stuff the files endup in an arbitrary location.
Not somewhere you can reliably find them.

m-wells · 2019-10-18T11:39:55Z

@oxinabox I created a pull request #6 to package julia-ctags into JuliaCtags (to stick with the Julia package naming conventions). It isn't much but it provides a method to copy the ctags config file into the current directory (naming the new file julia_ctags by default). If a ctags install was shipped with this package (see my comment in the pull request) other methods could be implemented that allow users to generate tags files using the package.

In addition, optionally providing a ctags install also means testing this package becomes much easier on Travis (or any other CI platform).

janEbert · 2019-10-18T17:36:32Z

Hey Karl, thank you so much for finding that. I actually hadn't forgotten this PR but due to the CI failing and me being so ashamed of the test code (it doesn't seem "right" at all), I never cared finishing it... Sorry! 😅

I will fix it later so #6 can be merged as well. By the way, I also wrote an Etags file for use with the Emacs-internal tags engine. Do you want it in the repo as well?

Correspondingly, add the generated "tags" file to .gitignore. I tried to add "actual code" and have too many tests rather than only complex ones. Surely, some cases are not covered. For example, using multiple macros for a definition.

Many tests are still marked as broken. The test/testtags file contains the desired tags to obtain. Some need only a little work, some need a more complete parser than the current argument-regex-based approach. We could possibly use the master of Universal Ctags for MacOS but that would introduce another error surface.

janEbert · 2019-10-24T21:23:41Z

Sorry it took so long. I also remembered why I didn't add the ctags executable in the first place. It was to avoid incompatibilities in case etags, often mapped to ctags, was found. Of course, due to the possibility of simply ordering the binaries, that didn't make sense. Also, that while-loop was pretty embarrassing, oh man... Good it's gone now. :)

karlwessel

Thanks for taking the time to write those tests! I added a few comments on the current commit.

karlwessel · 2019-10-27T14:56:35Z

test/test_tags.jl

+
+"Ctags binaries to sequentially try to call if one is not found."
+const ctagsbins = (
+        "uctags",


You should change the order of the possible tag command in such a way that the tag command available at your own system is last in the list. This way your make sure that you test the error case of the trygeneratetags method.

It would probably also be a good idea to only add those tag binaries to the list for which you can actually execute the tests. It is better to have a short but testes list of binaries then a complete but untested list.

People using binaries not in the list can then add their binary to the list them self, see if the test work and create an according PR or issue if necessary.

karlwessel · 2019-10-27T15:02:13Z

test/test_tags.jl

+            return generatetags(testfile, ctagsbinary)
+        catch e
+            # For compatibility accept either exception type.
+            if !(err isa LoadError || err isa ErrorException


For uctags the generatetags method failed with a Base.IOError instead of a LoadError (although from the printed error message it looked like a LoadError), so you should add that one to the list of 'acceptable' error.

I'm not sure if adding LoadError to the list of acceptable Exceptions is necessary. At least for my setup none of the failing tag binaries did throw an actual LoadError.

I expected ProcessFailedException

Sorry, I was busy when pushing and didn't test. I'll be sure to fix it, thanks again, Karl!
I actually can't remember why I included the LoadError (should have documented that one) but I'm sure it's there for a reason. Maybe I can replicate it again. But yeah if the IOError looked like a LoadError that may be the reason.

Hey Lyndon, that one's listed one line below, you can't see it in the diff.

ah cool cool

oxinabox reviewed Jul 29, 2019

View reviewed changes

Project.toml Outdated Show resolved Hide resolved

oxinabox reviewed Jul 30, 2019

View reviewed changes

test/runtests.jl Outdated Show resolved Hide resolved

oxinabox mentioned this pull request Jul 30, 2019

Update tags for Julia 1.0; fix #3 #4

Merged

janEbert force-pushed the tests branch from e1aa86f to 962733b Compare July 30, 2019 20:59

oxinabox reviewed Oct 17, 2019

View reviewed changes

test/test_tags.jl Show resolved Hide resolved

janEbert force-pushed the tests branch from 962733b to 4551bfc Compare October 24, 2019 21:15

Add test file

84a6aa7

Correspondingly, add the generated "tags" file to .gitignore. I tried to add "actual code" and have too many tests rather than only complex ones. Surely, some cases are not covered. For example, using multiple macros for a definition.

janEbert force-pushed the tests branch from 4551bfc to 06e6900 Compare October 24, 2019 21:20

janEbert force-pushed the tests branch from 06e6900 to a92bcbd Compare October 24, 2019 21:22

karlwessel suggested changes Oct 27, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add test file #5

Add test file #5

janEbert commented Jul 29, 2019

oxinabox commented Jul 29, 2019

janEbert commented Jul 29, 2019

oxinabox commented Jul 29, 2019

oxinabox commented Jul 29, 2019 •

edited

Loading

janEbert commented Jul 29, 2019 •

edited

Loading

oxinabox commented Jul 29, 2019

janEbert commented Jul 29, 2019 •

edited

Loading

janEbert commented Jul 30, 2019

oxinabox commented Jul 30, 2019

janEbert commented Jul 30, 2019 •

edited

Loading

janEbert commented Jul 30, 2019

karlwessel commented Oct 16, 2019

m-wells commented Oct 17, 2019

oxinabox commented Oct 17, 2019

m-wells commented Oct 17, 2019

oxinabox commented Oct 17, 2019

m-wells commented Oct 18, 2019

janEbert commented Oct 18, 2019

janEbert commented Oct 24, 2019 •

edited

Loading

karlwessel left a comment

karlwessel Oct 27, 2019

karlwessel Oct 27, 2019

oxinabox Oct 27, 2019

janEbert Nov 1, 2019 •

edited

Loading

oxinabox Nov 1, 2019

Add test file #5

Are you sure you want to change the base?

Add test file #5

Conversation

janEbert commented Jul 29, 2019

oxinabox commented Jul 29, 2019

janEbert commented Jul 29, 2019

oxinabox commented Jul 29, 2019

oxinabox commented Jul 29, 2019 • edited Loading

janEbert commented Jul 29, 2019 • edited Loading

oxinabox commented Jul 29, 2019

janEbert commented Jul 29, 2019 • edited Loading

janEbert commented Jul 30, 2019

oxinabox commented Jul 30, 2019

janEbert commented Jul 30, 2019 • edited Loading

janEbert commented Jul 30, 2019

karlwessel commented Oct 16, 2019

m-wells commented Oct 17, 2019

oxinabox commented Oct 17, 2019

m-wells commented Oct 17, 2019

oxinabox commented Oct 17, 2019

m-wells commented Oct 18, 2019

janEbert commented Oct 18, 2019

janEbert commented Oct 24, 2019 • edited Loading

karlwessel left a comment

Choose a reason for hiding this comment

karlwessel Oct 27, 2019

Choose a reason for hiding this comment

karlwessel Oct 27, 2019

Choose a reason for hiding this comment

oxinabox Oct 27, 2019

Choose a reason for hiding this comment

janEbert Nov 1, 2019 • edited Loading

Choose a reason for hiding this comment

oxinabox Nov 1, 2019

Choose a reason for hiding this comment

oxinabox commented Jul 29, 2019 •

edited

Loading

janEbert commented Jul 29, 2019 •

edited

Loading

janEbert commented Jul 29, 2019 •

edited

Loading

janEbert commented Jul 30, 2019 •

edited

Loading

janEbert commented Oct 24, 2019 •

edited

Loading

janEbert Nov 1, 2019 •

edited

Loading