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

Show file path URI in tracing messages #665

Merged
merged 7 commits into from
Oct 23, 2022

Conversation

jbriales
Copy link
Contributor

I'm considering a change to how the test error locations are printed in the trace,
so that one can jump to the location in an IDE with typical support.
We could make this a CLI option maybe, to keep things the same by default?

The new output would be:

 ✗ Prints full path URI with line location on error
   (in test file file:///home/jesusbriales/.dotfiles/test/misc/bats.bats:181)

instead of the current

 ✗ Prints full path URI with line location on error
   (in test file ./misc/bats.bats, line 181)

Before jumping into adding tests and any other extra work,
is this sth that you could consider merging to master?

@jbriales jbriales requested a review from a team as a code owner October 11, 2022 19:23
@martin-schulze-vireso
Copy link
Member

I have been thinking about this for some time now. I think we should allow to parametrize this with a flag. There might be other formats as well.

@jbriales
Copy link
Contributor Author

Would you have any suggestions or hints on how to expose this as a flag?
I managed to find the relevant logic for the feature, but I'm not familiar with how flags cascade from the UI deep to this level :)

@martin-schulze-vireso
Copy link
Member

Would you have any suggestions or hints on how to expose this as a flag? I managed to find the relevant logic for the feature, but I'm not familiar with how flags cascade from the UI deep to this level :)

Well, the flags are passed on again and again from (libexec/bats-core/) bats to bats-exec-suite to bats-exec-file to bats-exec-test. The parsing code consists of large case statements at the beginning of the files. If I find the time, I'll get around to do this myself eventually.

@martin-schulze-vireso martin-schulze-vireso added Type: Enhancement Priority: High Broken behavior in specific environments like in parallel mode or only on some operating systems Component: CLI Command line flags, exit code handling, ... Size: Large Changes across several files labels Oct 19, 2022
@martin-schulze-vireso martin-schulze-vireso added this to the 1.9.0 milestone Oct 19, 2022
@martin-schulze-vireso martin-schulze-vireso force-pushed the trace_file_uri branch 2 times, most recently from 2532714 to d117126 Compare October 21, 2022 21:53
martin-schulze-vireso added a commit to jbriales/bats-core that referenced this pull request Oct 21, 2022
@martin-schulze-vireso
Copy link
Member

@jbriales Do you have feedback about the naming?

  • --file-reference-format/BATS_FILE_REFERENCE_FORMAT
  • custom, comma_line, colon_separated, url_realpath

I also had to work around missing realpath on MacOS.

@jbriales
Copy link
Contributor Author

@jbriales Do you have feedback about the naming?

  • --file-reference-format/BATS_FILE_REFERENCE_FORMAT
  • custom, comma_line, colon_separated, url_realpath

Sweet! Naming is hard, but I think it's good like that :D
I wondered for a moment if uri vs url was more appropriate here. I thought general non-web file://-like URLs tended to be referred to more as URIs (vs URLs). But after checking online, I'm not so sure of the difference anymore :)

Also I would consider if calling it url/uri only, vs url_realpath. What's the current expected behavior wrt e.g. symlinks?
If we keep it simple and just return a full absolute path w/o taking further care for e.g. symlinks (as realpath does by default IIRC), then maybe just url (or url_abspath) would be more appropriate?

@martin-schulze-vireso
Copy link
Member

martin-schulze-vireso commented Oct 23, 2022

Okay, I renamed:

  • --file-reference-format -> --line-reference-format
  • BATS_FILE_REFERENCE_FORMAT -> BATS_LINE_REFERENCE_FORMAT
  • colon_separeted -> colon
  • url_realpath -> uri

@martin-schulze-vireso martin-schulze-vireso merged commit ca5a2dc into bats-core:master Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: CLI Command line flags, exit code handling, ... Priority: High Broken behavior in specific environments like in parallel mode or only on some operating systems Size: Large Changes across several files Type: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants