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

Bugfixes on tests and github actions runner for Windows #10196

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented May 12, 2020

This PR makes automatic testing on Windows working on github-actions.
It requires a bunch of bugfix/improvements:

  • Fixed an illegal operation in ant (even if it targets MasOSX the error is triggered on the Windows version of ant used on gh actions)
  • Disabled CRLF mangling on some files (git adds OS specific line endings when checking out, this is not good for tests that requires exact matching files)
  • String encoding by default is not UTF-8 on Windows. This affects SerialTest
  • CommandLineTests requires full stdin/out draining otherwise the Process.wait() command will wait forever...
  • msvc DLL are now placed with the bundled java executable, this way they are found even if arduino_debug.exe is launched from another folder
  • replaced FEST with assertJ (this makes finally the GUI tests passing on github actions, BTW I still see random failures that goes away by re-running the tests)

Copy link
Collaborator

@matthijskooijman matthijskooijman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I hope I didn't cause too much conflicts for you with my recent pullrequests ;-)

I left some small comments inline.

app/test/processing/app/SerialTest.java Show resolved Hide resolved
<classpathentry kind="lib" path="test-lib/assertj-swing-junit-1.2.0.jar"/>
<classpathentry kind="lib" path="test-lib/assertj-swing-junit-4.5-1.2.0.jar"/>
<classpathentry kind="lib" path="test-lib/fest-util-1.2.5.jar"/>
<classpathentry kind="lib" path="test-lib/junit-4.5.jar"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to downgrade junit from 4.11 to 4.5. Is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a forced dependency from assertj-swing-junit-4.5-1.2.0.jar, unfortunately I didn't found an assertj-swing-junit-4.11-1.2.0.jar.

Maybe it's ok to build with juint-4.11, but since it's not a critical dependency I've just downgraded it.

.github/workflows/unit-test.yml Show resolved Hide resolved
app/test/processing/app/CommandLineTest.java Show resolved Hide resolved
build/build.xml Show resolved Hide resolved
@facchinm facchinm added this to the Release 1.8.13 milestone Jun 3, 2020
CI run on windows produce the following error:

D:\a\Arduino\Arduino\build\build.xml:365: java.nio.file.InvalidPathException: Illegal char <*> at index 42: D:\a\Arduino\Arduino\build\macosx\arduino-*.dmg
Previously the <input...> tag was removed with sed to avoid hitting enter
on the version number prompt. We can just skip the dist step altogether
instead.
It looks like that on Windows string builders use a default encoding that
is different from UTF-8.
This allows the library loader to find them even if arduino_debug.exe or
arduino.exe is started from a different directory for example from the
command prompt.
Previously while testing on Windows, if the process produces a lot of
output that is not consumed, the pr.Wait() call will never end
because the OS doesn't have enough buffering.
For some reason the consolePane.getText() method returns the document
text with single CR translated into OS native CR+LF. This will influence
test outcome since we check various combinations of CR and LF.
@arduino arduino deleted a comment from ArduinoBot Jun 19, 2020
Wait for consumer thread to collect all output before returning from
command line run. This patch avoid a race condition (the test output
is checked before all the output is actually collected).
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.

None yet

4 participants