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

Make tests platform independent #202

Closed
wants to merge 7 commits into from
Closed

Make tests platform independent #202

wants to merge 7 commits into from

Conversation

davemaul
Copy link
Contributor

I managed to get the tests running with the default Maven structure (src/test/resources). Unfortunately, I had to copy UrlUtilclass into each project because Maven doesn't provide test code in the fluentlenium-core dependency.

Could you please verify the changes?

public class UrlUtil {

private UrlUtil() {
throw new UnsupportedOperationException("No instances allowed");
Copy link
Member

Choose a reason for hiding this comment

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

making this class final should be OK without this exception.

@Toilal
Copy link
Member

Toilal commented Dec 14, 2015

Please indent with 4 spaces instead of tabs

@Toilal
Copy link
Member

Toilal commented Dec 14, 2015

I think we should get rid of the constants defined in LocalFluentCase.java, and just call getAbsoluteUrlFromFile directly from concrete tests. This would make them more readable ...

@Toilal
Copy link
Member

Toilal commented Dec 14, 2015

But tests are running fine on my computer (ubuntu)

@davemaul
Copy link
Contributor Author

I use Eclipse with spaces only formatter. It's awkward why Git thinks, there's a difference between yours and mine. Is it some Linux vs. Windows thing?

@davemaul
Copy link
Contributor Author

Don't know if getting rid of the constants defined in LocalFluentCase is useful. What happens if the HTML file name changes? You would have to adapt all tests using the file instead of changing it in one place. However, this should be done in another pull request if necessary.

@davemaul
Copy link
Contributor Author

I think, Travis crashed. 😅

*** buffer overflow detected ***: /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java terminated
It's the openjdk7 build.

@Toilal
Copy link
Member

Toilal commented Jan 4, 2016

Thanks @logineimer. Is the work over with this branch ? I'll try to give a try tomorrow.

@Toilal
Copy link
Member

Toilal commented Jan 5, 2016

I've rebased everything in a single commit because commit history was weird, with branching and merging ... But it's merged, thanks.

@Toilal Toilal closed this Jan 5, 2016
@davemaul davemaul deleted the feature/reorganize-tests branch March 4, 2016 10:17
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

2 participants