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

Pragmatic displayed impl #445

Merged
merged 4 commits into from
May 26, 2022

Conversation

daveyarwood
Copy link
Contributor

This hopefully fixes #444.


Unfortunately, I wasn't able to run the tests due to a variety of technical issues.

I don't have Docker on this machine, so I attempted running bb test all directly.

I got an error about the Cognitect test runner library missing :sha from the coordinate, which I've read can be caused by using an older version of the Clojure CLI. But then I updated to the latest Clojure CLI version and the problem persisted. I was able to work around that by adding an additional :sha entry for all deps that have a :git/sha entry.

Having worked that out, I ran bb test all and got this error which I don't understand:

$ bb test all

[ TASK test all ]----------------------------------------------------------------

[ Running all tests on jvm ]-----------------------------------------------------
Error building classpath. Cannot invoke "java.lang.CharSequence.length()" because "this.text" is null
java.lang.NullPointerException: Cannot invoke "java.lang.CharSequence.length()" because "this.text" is null
        at java.base/java.util.regex.Matcher.getTextLength(Matcher.java:1769)
        at java.base/java.util.regex.Matcher.reset(Matcher.java:415)
        at java.base/java.util.regex.Matcher.<init>(Matcher.java:252)
        at java.base/java.util.regex.Pattern.matcher(Pattern.java:1134)
        at java.base/java.util.regex.Pattern.split(Pattern.java:1262)
        at java.base/java.util.regex.Pattern.split(Pattern.java:1335)
        at clojure.string$split.invokeStatic(string.clj:224)
        at clojure.string$split.invoke(string.clj:219)
        at clojure.tools.gitlibs.impl$clean_url.invokeStatic(impl.clj:52)
        at clojure.tools.gitlibs.impl$clean_url.invoke(impl.clj:48)
        at clojure.tools.gitlibs.impl$git_dir.invokeStatic(impl.clj:59)
        at clojure.tools.gitlibs.impl$git_dir.invoke(impl.clj:57)
        at clojure.tools.gitlibs.impl$ensure_git_dir.invokeStatic(impl.clj:83)
        at clojure.tools.gitlibs.impl$ensure_git_dir.invoke(impl.clj:80)
        at clojure.tools.gitlibs$procure.invokeStatic(gitlibs.clj:67)
        at clojure.tools.gitlibs$procure.invoke(gitlibs.clj:61)
        at clojure.tools.deps.alpha.extensions.git$eval1356$fn__1358.invoke(git.clj:42)
        at clojure.lang.MultiFn.invoke(MultiFn.java:239)
        at clojure.tools.deps.alpha$expand_deps.invokeStatic(alpha.clj:422)
        at clojure.tools.deps.alpha$expand_deps.invoke(alpha.clj:390)
        at clojure.tools.deps.alpha$resolve_deps.invokeStatic(alpha.clj:495)
        at clojure.tools.deps.alpha$resolve_deps.invoke(alpha.clj:475)
        at clojure.tools.deps.alpha$calc_basis.invokeStatic(alpha.clj:655)
        at clojure.tools.deps.alpha$calc_basis.invoke(alpha.clj:629)
        at clojure.tools.deps.alpha.script.make_classpath2$run_core.invokeStatic(make_classpath2.clj:91)
        at clojure.tools.deps.alpha.script.make_classpath2$run_core.invoke(make_classpath2.clj:57)
        at clojure.tools.deps.alpha.script.make_classpath2$run.invokeStatic(make_classpath2.clj:119)
        at clojure.tools.deps.alpha.script.make_classpath2$run.invoke(make_classpath2.clj:113)
        at clojure.tools.deps.alpha.script.make_classpath2$_main.invokeStatic(make_classpath2.clj:169)
        at clojure.tools.deps.alpha.script.make_classpath2$_main.doInvoke(make_classpath2.clj:140)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.lang.Var.applyTo(Var.java:705)
        at clojure.core$apply.invokeStatic(core.clj:667)
        at clojure.main$main_opt.invokeStatic(main.clj:514)
        at clojure.main$main_opt.invoke(main.clj:510)
        at clojure.main$main.invokeStatic(main.clj:664)
        at clojure.main$main.doInvoke(main.clj:616)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.lang.Var.applyTo(Var.java:705)
        at clojure.main.main(main.java:40)
----- Error --------------------------------------------------------------------
Type:     java.io.FileNotFoundException
Message:  .cpcache/6014BBA0687CD475538344CF774A2803.cp (No such file or directory)
Location: 23:1

I hope it's OK if I submit this PR anyway. If you have CI hooked up to run the tests, or if a volunteer could check out my branch and run them, that would be helpful! If the tests aren't passing, I can try to dig deeper.

@daveyarwood
Copy link
Contributor Author

Looks like the tests passed! 🎉

@lread
Copy link
Collaborator

lread commented May 26, 2022

@daveyarwood: As for testing locally, I've not seen that error yet myself.

  • Did you try deleting .cpcache dir?
  • Is your bb version up to date?

@daveyarwood
Copy link
Contributor Author

Is your bb version up to date?

Aha! That was the issue. I was running an old version of Babashka, 0.4.4. I've updated to the latest and now the tests are running. 👍

@borkdude
Copy link
Contributor

@lread Maybe specify :min-bb-version "0.8.2" in bb.edn.

@daveyarwood
Copy link
Contributor Author

Most of the tests passed, but some of them (looks to be tests related to window size/position) failed. I'm attaching the log here in case it's helpful.

test-output.log

@lread
Copy link
Collaborator

lread commented May 26, 2022

Thanks @daveyarwood! Can you run bb tools-versions for me and paste the results?

@daveyarwood
Copy link
Contributor Author

$ bb tools-versions

[ TASK tools-versions  ]---------------------------------------------------------
|-------------------+-----------------------------------------------------------------------------------------------------+---------------------------------|
|        Name       |                                               Version                                               |               Path              |
|-------------------+-----------------------------------------------------------------------------------------------------+---------------------------------|
| Java              | openjdk version "17.0.3" 2022-04-19                                                                 | /home/dave/.jenv/shims/java     |
|                   | OpenJDK Runtime Environment (build 17.0.3+7-Ubuntu-0ubuntu0.20.04.1)                                |                                 |
|                   | OpenJDK 64-Bit Server VM (build 17.0.3+7-Ubuntu-0ubuntu0.20.04.1, mixed mode, sharing)              |                                 |
| Babashka          | babashka v0.8.2                                                                                     | /home/dave/bin/bb               |
| Image Magick      | Version: ImageMagick 6.9.10-23 Q16 x86_64 20190101 https://imagemagick.org                          | /usr/bin/identify               |
| Chrome            | Google Chrome 102.0.5005.61                                                                         | /usr/bin/google-chrome          |
| Chrome Webdriver  | ChromeDriver 101.0.4951.41 (93c720db8323b3ec10d056025ab95c23a31997c9-refs/branch-heads/4951@{#904}) | /home/dave/npm/bin/chromedriver |
| Firefox           | Mozilla Firefox 100.0.2                                                                             | /usr/bin/firefox                |
| Firefox Webdriver | geckodriver 0.30.0                                                                                  | /usr/bin/geckodriver            |
|-------------------+-----------------------------------------------------------------------------------------------------+---------------------------------|

TASK tools-versions done.

@lread
Copy link
Collaborator

lread commented May 26, 2022

That all looks good to me @daveyarwood.
The proc tests are a bit delicate I think, and might suffer if any WebDriver processes are already running.
There's bb drivers to list bb drivers kill to turf them.

Not sure about your window size/positioning failures.
Will maybe try to reproduce them on a linux box later.

@lread
Copy link
Collaborator

lread commented May 26, 2022

Re-running the failed job. It seems to be not unusual for a job or two to sometimes fail on GitHub Actions. If they still fail after a re-run then we likely have an issue to deal with (which is not the case here I am pretty certain!)

@lread
Copy link
Collaborator

lread commented May 26, 2022

@daveyarwood, I raised an issue for your local failure #447

@lread lread merged commit 564c86c into clj-commons:master May 26, 2022
@daveyarwood daveyarwood deleted the pragmatic-displayed-impl branch May 26, 2022 19:38
@daveyarwood daveyarwood restored the pragmatic-displayed-impl branch May 26, 2022 19:38
@daveyarwood daveyarwood deleted the pragmatic-displayed-impl branch May 26, 2022 19:39
lread added a commit to lread/etaoin that referenced this pull request Aug 1, 2022
We have learned that displayed-ness is more complex than we had
originally imagined. It requires more research and maybe more hammock
time.

Effectively rolls back PR clj-commons#445 for issue clj-commons#444.
lread added a commit that referenced this pull request Aug 1, 2022
We have learned that displayed-ness is more complex than we had
originally imagined. It requires more research and maybe more hammock
time.

Effectively rolls back PR #445 for issue #444.
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.

Consider etaoin.api/displayed? behavior
3 participants