Including scripts by relative reference - Fixes #303

[aartiPl]

This patch Fixes #303

I have tested it on my branch with URLs in the includes directory directed to my branch. After that, I have reverted them back to point to your master branch. Because of that, tests will not work on the branch, but only on the master.

I have started only Tests class in src/test directory. I am not sure how to start other tests. Please let me know if something else should be done to merge this PR.

[holgerbrandl]

Awesome. Thank you so much for supporting script!

I've run the test but they still seems to struggle with the PR:

Tests > test_include_annotations FAILED
org.junit.ComparisonFailure: expected:<...ion
fun url_included[_1() = println("i came from the internet")
fun url_included_2() = println("i also] came from the inter...> but was:<...ion
fun url_included[() = println("i] came from the inter...>
at io.kotlintest.matchers.MatchersKt.shouldBe(Matchers.kt:15)
at Tests.test_include_annotations(Tests.kt:212)

Tests > test_include_detection FAILED
java.lang.AssertionError: expected: 2 but was: 1
at io.kotlintest.matchers.MatchersKt.equalsError(Matchers.kt:89)
at io.kotlintest.matchers.MatchersKt.shouldEqual(Matchers.kt:63)
at io.kotlintest.matchers.MatchersKt.shouldBe(Matchers.kt:55)
at Tests.test_include_detection(Tests.kt:220)

[aartiPl]

There is an inherent problem with testing changes covering resolving URLs on branches, at least in the current test setup. Test files rely on KScript Github master branch repository. But the problem is that if you make changes in test files on a branch, then those files are not yet in the master branch of the Github repo, so the tests will just fail.

In case of this PR, there is added a new script file, which is included by URL in test/resources/includes/rel_includes/include_by_url.kt. This new file is obviously not in the master branch, because PR is not yet merged, and that's why tests on the branch fail.

In such a case, to accomplish the tests on the PR branch you can do the following things:

1. Modify URLs to the test branch in files:
   test/resources/includes/include_by_url.kt
   test/resources/includes/include_variations.kts

For testing in my forked branch I have used the following URLs:
include_by_url.kt: @file:Include("https://raw.githubusercontent.com/aartiPl/kscript/Including_scripts_by_relative_reference_-_fixes_%23303/test/resources/includes/rel_includes/include_by_url.kt")
include_variations.kts: @file:Include("https://raw.githubusercontent.com/aartiPl/kscript/Including_scripts_by_relative_reference_-_fixes_%23303/test/resources/includes/include_by_url.kt")

2. Otherwise the tests can be run successfully only on the "master" branch, so only after the merge of PR.

[aartiPl]

Hi! I have improved a patch a little bit. Now it is disallowed to include local files from scripts that are invoked by HTTP reference. IMHO that would be a security issue. Additionally, I have modified the URLs in the test resources, so that tests are passing now on this branch. Please modify them back to the master branch before the merge.

Please let me know your comments about the patch.

[aartiPl]

Hi!
Is there a chance to proceed with this PR?

Lack of this change blocks from creating real script libraries. That's why I have worked on this PR - I need the changes in my work to create a library of reusable scripts.

Please let me know if I can help somehow to close PR and release a new version of the kscript.
Thanks!

[holgerbrandl]

I'm really sorry for the delay.

[holgerbrandl]

Not sure if it's PR or build-system related, but tests still don't seem fine. See build status from above.

[holgerbrandl]

Not sure if it's PR or build-system related, but tests still don't seem fine. See build status from above.

[aartiPl]

That's okay. But I think we will have to discuss why it fails, because it definitely passes on my computer:

I suspect that it is something about the branches: currently on aartiPl:Including_scripts_by_relative_reference_-fixes#303 branch test files are pointing to the test resources on the branch. As I noted before they will fail on the master branch. If this is the issue - it is not simply solvable. I have tried to describe the issue in details in my previous comments. Do you understand what is the root cause of the problem? I am happy to meet on the Zoom or Google Meet and shortly explain the issue. My private email is: aarti@interia.pl - please drop me a message and we will set up a short meeting.

If that's something else, then I don't know how to reproduce it. In such a case I would be happy to understand why it happens. A short meeting will also help.

[aartiPl]

And it looks rather like something with the test suite:

First test which has failed is the simplest possible one, which was not modified in any way:

[aartiPl]

Ok. I was able to start test_suite.sh. I will recheck on my side :-)

[aartiPl]

I believe the errors are related to problems with the test script itself. I have refactored a little bit test_suite.sh, so that it does not need $KSCRIPT_HOME environment variable to execute itself. (I have changed it because sdkman is setting this variable to a completely different location and I don't think it is a good idea to mess with this env var if not needed). So there are currently two local variables in the test script: SCRIPT_DIR and PROJECT_DIR, which are calculated automatically, regardless of where the script is located. I have also improved logging in the test_suite.sh a little bit.

Below you can find the execution logs from my test run:

`
$ ./test_suite.sh
Starting KScript test suite...
Script dir : /home/vagrant/workspace/Kod/Repos/kscript/test
Project dir: /home/vagrant/workspace/Kod/Repos/kscript

Starting script input modes tests:
...................
all 19 script_input_modes tests passed in 31.308s.

Starting cli helper tests:

Starting environment tests:
...
all 3 environment_tests tests passed in 2.353s.

Starting dependency lookup tests:
......
all 6 dependency_lookup tests passed in 9.455s.

Starting annotation-driven configuration tests:
......
all 6 annotation_config tests passed in 5.745s.

Starting support api tests:
.....
all 5 support_api tests passed in 4.717s.

Starting kt support tests:
.......XX
Starting custom interpreters tests:
..
test #8 "rm -f /home/vagrant/workspace/Kod/Repos/kscript/test/package_example && kscript --package /home/vagrant/workspace/Kod/Repos/kscript/test/resources/package_example.kts &>/dev/null && /home/vagrant/workspace/Kod/Repos/kscript/test/package_example 1" failed:
expected "package_me_args_1_mem_5368709120"
got nothing
test #9 "rm -f kscriptlet* && cmd=$(kscript --package "println(args.size)" 2>&1 | tail -n1 | cut -f 5 -d " ") && $cmd three arg uments" failed:
expected "3"
got nothing
2 of 11 custom_interpreters tests failed in 15.389s.

Starting misc tests:
........X.X
test #9 "tmpDir=$(kscript_nocall --idea ${PROJECT_DIR}/test/resources/includes/include_variations.kts | cut -f2 -d" " | xargs echo); cd $tmpDir && gradle build" failed:
program terminated with code 1 instead of 0
test #11 "tmpDir=$(kscript_nocall --idea ${PROJECT_DIR}/test/resources/includes/diamond.kts | cut -f2 -d" " | xargs echo); cd $tmpDir && gradle build" failed:
program terminated with code 1 instead of 0
2 of 11 misc tests failed in 10.443s.

Starting junit suite tests:
.
all 1 junit_tests tests passed in 2.318s.

Starting bootstrap headers tests:
.....
all 5 bootstrap_header tests passed in 3.819s.
`

As you can see there are only four tests, which are failing, totally unrelated to my changes. The first two are connected with the packaging option in kscript (I don't understand very well what they do) and the other two are connected with IntelliJ idea integration, which I don't use also. The failures might be connected with different versions of Gradle or by using relative paths instead of absolute. I can't test it without much fuss, so if you could look at it, it will be appreciated.

[holgerbrandl]

Packaging seems indeed broken currently (see #324).

I've run the tests locally against the PR and struggle to complete "kscript https://git.io/fxHBv" both on the terminal and from the IDE. This is because the file-suffix is no longer detected correctly. It must be kt but seems to be kts with PR. Am I missing something here?
Make sure to always wipe the cache before testing.

[aartiPl]

There is indeed something wrong with my implementation. I will try to figure out what's that and provide a fix. Thanks for your input!

[aartiPl]

Edit: there is one more case to fix. I will work on that...

Ok. I have managed to fix the problem. The remaining problems are with:

• packages - seems that they are broken anyway
• idea project - I can not test it as I am working in a virtual machine without UI

Please check if idea generated project is working as expected.

It seems to me that there would be needed some deeper refactoring in KScript. I have an idea how to do that. If you are willing to do such changes, I can work on it. But I prefer to do this after applying changes from this PR first.

Please do not forget to change links in test files before the merge to the master:
#330 (comment)

[aartiPl]

Seems that the last broken use case is fixed now. There are failures only with packaging and idea. Can you please check those use cases?

[holgerbrandl]

Thank you so much @aartiPl for your patience with my slow response times, and many thanks for your great support.