bash script which tests that amberc command generates the js file

[Exocen]

Hello,

This script only tests the js file generation, we'll complete with further tests later.

Thanks.

Quentin, Célia, Jonathan, Romain

[mkroehnert]

Since amberc is written in JavaScript, wouldn't it make more sense to hook into the JS routines and test those separately (maybe with Mock objects to get around the filesystem things)?
This would also easy the possibility to test intermediate steps.
And changes to the compiler which are required to make it better testable in this way should be done anyway.

Also, Bash scripts make it really hard to run those test on Windows platforms where they should definitely be run.

[ghost]

Well, #991 tells specifically "functional tests", so these, end-to-end tests were in mind.

Manfred Kröhnert wrote:

Since |amberc| is written in JavaScript, wouldn't it make more sense
to hook into the JS routines and test those separately (maybe with
Mock objects to get around the filesystem things)?
This would also easy the possibility to test intermediate steps.
And changes to the compiler which are required to make it better
testable in this way should be done anyway.

—
Reply to this email directly or view it on GitHub
#1095 (comment).

[hhzl]

Working with Amber needs git and the recommendation for Microsoft Windows is to go for msysgit. It includes bash. Depending on the script (usage of the environment) it might work there as well

[ghost]

Yes, that is possible, but hackish and hard to achieve cleanly. The next step would be changeing it from bash to something else (shelljs seems like easy step to make it run platform-independently).

Hannes Hirzel wrote:

Working with Amber needs git and the recommendation for Microsoft
Windows is to go for msysgit http://msysgit.github.com/. It includes
|bash|. Depending on the script (usage of the environment) it might
work there as well

—
Reply to this email directly or view it on GitHub
#1095 (comment).

[hhzl]

It depends on how large the subset of bash commands used for this test is. It is probably quite small. The subset used by shelljs is small as well.

OTHO a better integration with the rest of the Amber environment (i.e. nodejs) is desirable.

In any case a bash script is a welcome addition to the documentation as it shows how to drive an Amber app from another scripting language.

[ghost]

Yes, of course. I looked into the test and it really uses just a few basic things, so it is shelljsizable fine.

[hhzl]

@Herby Not sure about the need for shelljs at this moment. It will run fine on your Microsoft Windows machine as well. :)

[ghost]

Of course it is needed. Canonical way to run all tests is 'npm test' (automatically used by travis for node projects; it calls 'grunt test' for amber). If you try to make this work on both unixes and windows, good luck with all the hacks.

I don't care about possibility to run this single test (that I can do in git bash, of course). I care about their inclusion in overall test suite. That is what those issues are for - make tests for xxx so it can be included to automatic testing. I did not stress it because it was just natural to me. Maybe I should have done, though.

Hannes Hirzel wrote:

@Herby https://github.com/herby Not sure about the need for shelljs
at this moment. It will run fine on your Microsoft Windows machine as
well. :)

amber-issue1095-test-on-mswindows-ok
https://cloud.githubusercontent.com/assets/1112023/4647193/9c788a20-5479-11e4-81dd-34d0d10ed3b2.png

—
Reply to this email directly or view it on GitHub
<https://github.com/amber
-smalltalk/amber/pull/1095#issuecomment-59216579>.

[Exocen]

Here the correction of amberctest.sh (we failed a little with the 3 commits)

Quentin, Celia, Jonathan, Romain

[ghost]

Squash them, pls.

[mkroehnert]

Then I think it is okay, if it can be made functional on non-Bash platforms, too.

[mkroehnert]

I think it would be much cleaner if the files were stored in a temporary directory with a unique name.
This would reduce possible issues with existing files having the same name.
Additionally, it is then safe to remove the complete directory and not only select files.

[ghost]

There is still a lot of commits there (click graph icon, then network, you'll see there are still unneeded ones). Could you please, while having this branch checked out, rebase it (git rebase -i master, which should present you, in the text editor, with the list of all commits from the point the branching off master preceded by keyword pick) so that only the relevant ones are retained (8d7c776 and 4898f03; that is, you delete all other pick lines and save the file in editor)? After successful rebase, you should only have these two commits present in a branch (with different hash humbers, but with same contents; check with git log), and you can then git push -f to push the reconstructed branch to github, replacing old one with the new one. In fact, rebase and git push -f are big evils when done in master branch, never do that; but in feature branches, they are just normal bookkeeping tools. [By asking "please squash them" I asked for rebase + git push -f, actually (you can merge commits by using squash instead of pick, the process is called 'to squash'), but now, just selecting the two mentioned above is easier]..

(if you added more commits to the branch in the meantime, retain them in rebase as well, so you don't lose them, of course)

(if you are afraid of error, then before doing any rebase, you can just git checkout -b backup to create backup branch that remembers actual state, then git checkout issue991 and you can proceed with rebase; if the process failed somehow, just git rebase --abort, git checkout issue991 and git reset --hard backup to get to the initial state)

[CeliaDoolaeghe]

Hello.

Sorry to have not answered before.
In fact, we've finished the course already and we have too much work now, so we really don't have the time to translate it into shelljs now.

Sorry and thank you.

Celia, Jonathan, Quentin, Romain