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

Add functional tests #35

Merged
merged 4 commits into from
Jan 13, 2021
Merged

Conversation

johnboyes
Copy link
Contributor

@johnboyes johnboyes commented Jan 8, 2021

The functional tests are themselves Gauge tests.

The scaffolding for the tests has been lifted and shifted from the
gauge-tests repository. When adding more Spectacle functional tests
definitely browse the gauge-tests repository for inspiration and
ideas.

This commit only adds a small number of functional tests with limited
coverage of Spectacle's functionality, but as these are the first
functional tests for Spectacle there is still significant
value from a regression prevention perspective. Further tests can be
added in future commits if deemed useful.

The functional tests run on every push and pull request, triggered by
a GitHub Action.

Signed-off-by: John Boyes [email protected]

@johnboyes
Copy link
Contributor Author

Hi @zabil, I've created this PR as per our earlier discussion. Very grateful if you can take a look, please 🙂

The functional tests are themselves Gauge tests.

The scaffolding for the tests has been lifted and shifted from the
[gauge-tests][1] repository. When adding more Spectacle functional tests
definitely browse the `gauge-tests` repository for inspiration and
ideas.

This commit only adds a small number of functional tests with limited
coverage of Spectacle's functionality, but as these are the first
functional tests for Spectacle there is still significant
value from a regression prevention perspective.  Further tests can be
added in future commits if deemed useful.

The functional tests run on every push and pull request, triggered by
a GitHub Action.

[1]: https://github.com/getgauge/gauge-tests

Signed-off-by: John Boyes <[email protected]>
@johnboyes
Copy link
Contributor Author

Hi @zabil, v grateful indeed if you or one of the team is able to have a look at this whenever you have time, please - much appreciated if you have time.
I've got a fully working initial version of the Jira plugin that I've been working on waiting in the wings - am keen to get this PR over the line first, so that I can generate the Jira plugin repo using this Spectacle repo as a template, complete with functional tests :-)

@zabil
Copy link
Member

zabil commented Jan 12, 2021

Hey @johnboyes I was trying to run it locally and make sense of the files.
Here's a few things i notice. When I run the specs a follows

./gradlew clean javaFT

> Task :compileTestJava
Note: /Users/zabilcm/Gauge/spectacle/functional-tests/src/test/java/com/thoughtworks/gauge/test/common/ExecutionSummaryAssert.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Executing in 2 parallel streams.
[runner: 2] # Spectacle multiple specs in a folder
[runner: 1] # Spectacle spec with scenarios
[runner: 1]   ## Basic spec with one scenario
[runner: 1]   ## Basic spec with multiple scenarios
[runner: 2]   ## Document multiple specs in a folder

It only runs the specs related to spectacle which is great. However, there's step implementation around testing other langauge runners in the implemetation for example https://github.com/getgauge/spectacle/pull/35/files#diff-0e0a20cb6653d09e61a3f338a5c2dd395af8533e916d76a6fe2c757541f22d98R20

Is it possible to modify the PR and only have code related to testing the scenarios above?

Also it will be good to add the following to the .gitignore file

.gauge
.gradle
logs
reports-java
resources
testLogs
build

@johnboyes
Copy link
Contributor Author

Hi @zabil, thanks for the comment. I've updated the .gitignore in 2507dc3, and removed unused functional tests Java code in 3da5f55. I think it could be possible to remove more Java code, but that would start to entail unravelling the functional test framework code I would say (which is something I think we should avoid). Maybe what's that telling us is that in the future (rule of 3, maybe), it may be worthwhile extracting the functional test framework code into its own module - thoughts?

@zabil
Copy link
Member

zabil commented Jan 13, 2021

it may be worthwhile extracting the functional test framework code into its own module - thoughts?

I do see duplicate code, but I think that is fine. I think both these should evolve separately.

I made a few changes to this PR locally around removing env files and modifying the github action and readme. I can't seem to commit these changes to your branch due to permissions. Maintainers can push to the PR branch provided an option is checked while creating a PR https://tighten.co/blog/adding-commits-to-a-pull-request/

Can you enable that if you are ok? I could also make a patch and send it along.

@johnboyes
Copy link
Contributor Author

Thanks, I'd be happy to enable that setting but it doesn't work unfortunately in this case, because my fork is on my organisation account, not my individual account. So would definitely welcome a patch, please - sorry for the hassle!

* Use the default env configuration
* Only use the java langauge runner
* Modify github actions to use java

Signed-off-by: Zabil Cheriya Maliackal <[email protected]>
@johnboyes
Copy link
Contributor Author

Thanks for sending the patch through, @zabil, I've applied it (3f7ac9b) :-)

@zabil zabil merged commit 22279a4 into getgauge:master Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants