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

Fix 1134 #1135

Merged
merged 3 commits into from Nov 14, 2017

Conversation

Projects
None yet
3 participants
@JoshuaMcManus

JoshuaMcManus commented Nov 12, 2017

Description

Do statements for state machines did not output // line ## "Filename" comments. This fixes this by injecting the comment into the generated Java code.

Tests

Changes two tests to verify comments.

Fixes Issue #1134

Josh McManus added some commits Nov 12, 2017

Josh McManus
Fixes #1134
Do statements in state machines will now output a // line statement indicating where the source code is from
@vahdat-ab

This comment has been minimized.

Show comment
Hide comment
@vahdat-ab

vahdat-ab Nov 12, 2017

Member

@AlexJoshuaMcManus please check why your build is failing

Member

vahdat-ab commented Nov 12, 2017

@AlexJoshuaMcManus please check why your build is failing

@JoshuaMcManus

This comment has been minimized.

Show comment
Hide comment
@JoshuaMcManus

JoshuaMcManus Nov 12, 2017

@vahdat-ab it looks like the tests all passed in Eclipse, but when they're run in test suite they fail because the relative directory changes for the // line output.

How would you suggest I test my changes, if at all?

JoshuaMcManus commented Nov 12, 2017

@vahdat-ab it looks like the tests all passed in Eclipse, but when they're run in test suite they fail because the relative directory changes for the // line output.

How would you suggest I test my changes, if at all?

@vahdat-ab

This comment has been minimized.

Show comment
Hide comment
@vahdat-ab

vahdat-ab Nov 13, 2017

Member

@AlexJoshuaMcManus
I tried on my own machine, I faced some failures.

    [junit] TEST cruise.umple.statemachine.implementation.StateMachineTest FAILED
    [junit] TEST cruise.umple.statemachine.implementation.java.JavaStateMachineTemplateTest FAILED
    [junit] Tests FAILED

When I switch to master, it passes all test cases. It means there are issues in your branch.

Member

vahdat-ab commented Nov 13, 2017

@AlexJoshuaMcManus
I tried on my own machine, I faced some failures.

    [junit] TEST cruise.umple.statemachine.implementation.StateMachineTest FAILED
    [junit] TEST cruise.umple.statemachine.implementation.java.JavaStateMachineTemplateTest FAILED
    [junit] Tests FAILED

When I switch to master, it passes all test cases. It means there are issues in your branch.

@JoshuaMcManus

This comment has been minimized.

Show comment
Hide comment
@JoshuaMcManus

JoshuaMcManus Nov 13, 2017

@vahdat-ab

Sorry, I guess I wasn't clear. Yes I know the tests that I implemented are the ones that are failing. This is because I modified existing tests to look for the new comment that reads:
// line ## "path/to/file"

However, when the tests are run in Eclipse, the path to file is something like "../file.ump", but when the tests are run during a full build they look more like: "../../.../../cruise.umple/test/file.ump"

How would you suggest I go about fixing this? The tests are looking for the comments to match (since that is what I implemented), but it looks like the relative directory changes depending on how the test is called.

JoshuaMcManus commented Nov 13, 2017

@vahdat-ab

Sorry, I guess I wasn't clear. Yes I know the tests that I implemented are the ones that are failing. This is because I modified existing tests to look for the new comment that reads:
// line ## "path/to/file"

However, when the tests are run in Eclipse, the path to file is something like "../file.ump", but when the tests are run during a full build they look more like: "../../.../../cruise.umple/test/file.ump"

How would you suggest I go about fixing this? The tests are looking for the comments to match (since that is what I implemented), but it looks like the relative directory changes depending on how the test is called.

@vahdat-ab

This comment has been minimized.

Show comment
Hide comment
@vahdat-ab

vahdat-ab Nov 13, 2017

Member

I see.
The main priority is the command line. The server always runs tests cases through the command line.
w.r.t to the issue with Eclipse, I would say check other test cases in which we test the location and address of files. We have not had any issue with it.

Member

vahdat-ab commented Nov 13, 2017

I see.
The main priority is the command line. The server always runs tests cases through the command line.
w.r.t to the issue with Eclipse, I would say check other test cases in which we test the location and address of files. We have not had any issue with it.

Josh McManus
Changes the test to verify the comment
The previous test had a namespace specified, which meant that the output varied based on where the test was called (Eclipse and terminal had different build paths). Trying to change the namespace in that file only resulted in the file not being cleaned up
@JoshuaMcManus

This comment has been minimized.

Show comment
Hide comment
@JoshuaMcManus

JoshuaMcManus Nov 14, 2017

@vahdat-ab I've changed the test so it's now passing the build, but I noticed that 3/4 Travis builds are failing, is that supposed to happen? I remember always failing 2, but not 3.

JoshuaMcManus commented Nov 14, 2017

@vahdat-ab I've changed the test so it's now passing the build, but I noticed that 3/4 Travis builds are failing, is that supposed to happen? I remember always failing 2, but not 3.

@TimLethbridge TimLethbridge merged commit 1af66aa into master Nov 14, 2017

1 of 4 checks passed

continuous-integration/appveyor/branch AppVeyor build failed
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@TimLethbridge TimLethbridge deleted the fix-1134_jmcmanus branch Nov 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment