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

introduce gauge_data_dir #2047

Merged
merged 5 commits into from
Jul 28, 2021
Merged

introduce gauge_data_dir #2047

merged 5 commits into from
Jul 28, 2021

Conversation

sriv
Copy link
Member

@sriv sriv commented Jul 15, 2021

allow csv/txt data files to be in subdirectory, configured by gauge_data_dir

fixes #2046

Signed-off-by: sriv srikanth.ddit@gmail.com

allow csv/txt data files to be in subdirectory, configured by gauge_data_dir

fixes #2046

Signed-off-by: sriv <srikanth.ddit@gmail.com>
@gaugebot
Copy link

gaugebot bot commented Jul 15, 2021

@sriv Thank you for contributing to gauge. Your pull request has been labeled as a release candidate 🎉🎉.

Merging this PR will trigger a release.

Please bump up the version as part of this PR.

Instructions to bump the version can found at CONTRIBUTING.md

If the CONTRIBUTING.md file does not exist or does not include instructions about bumping up the version, please looks previous commits in git history to see what changes need to be done.

@sriv sriv marked this pull request as draft July 15, 2021 18:10
@alpha1592
Copy link

I tried out the sample project and the build 1.3.3.
Its looking good. I like it.

Couple of points that I noticed while playing around with this.

  • Gauge should output the path to the table being used during execution on the console.
    • This would help when troubleshooting issues.
    • I used the default log level, so not sure if this info is logged on debug or trace levels.
    • I think it should be logged on info level so at the start of test, user is clear where the data is coming from
    • Similarly, i think the path to the table should be logged in the html/other reports
  • When Gauge is unable to find the Table, the error is not clear.
    • I provided an invalid path to gauge_data_dir
    • gauge_data_dir = /env/bar/data2 instead of the correct gauge_data_dir = /env/bar/data
    • Error: Could not resolve table from table: bar.csv => 'table: bar.csv'
    • The error message should be clear as to where Gauge is looking for the table.
    • Example: Error: Gauge could not find the table at /env/bar/data2/bar.csv
  • This is a minor point, but there is no need for another property file data.properties
    • There are already too many property files...
    • You used data.properties to demonstrate the property gauge_data_dir in the sample project.
    • This was probably done to highlight the new property, but i just want to have it clarified that a new property file is not needed.
    • I put the property in default.properties and it works, Which is good.
    • Put in your documentation that this property can be put in any of the existing property files including default.properties and java.properties and gauge will pick it up.

Other than that, things look good. This will solve a lot of our problems.
I will play around some more (if I have time). I use Gauge with Java, Maven and Spring, so I will try to use this build with my own sample project.

When do you expect to have this feature released?

Now, if you can just add the table feature at the scenario level, it would be perfect.

This way, we can have different number of rows/different csv files for each scenarios if needed.

## This is a scenario
table: bar.csv

* Show me the <name> and the <email> address.

Thanks A lot! Great Work!

@sriv
Copy link
Member Author

sriv commented Jul 22, 2021

Thank you for the feedback. To answer your points:

Gauge should output the path to the table being used during execution on the console.

Yes, I'll add that in. I think this is best printed in debug level, since this information is probably needed when you need to troubleshoot. This will also be consistent on how gauge prints out other information like env properties in debug mode.

When Gauge is unable to find the Table, the error is not clear.

Absolutely, this is a good point. I'll add this information in the error message.

but there is no need for another property file data.properties

True. The properties files are split just to help organize better. You could have them all in a single file and gauge will still pick all the values. As with other properties, you can also set this as an env variable.

When do you expect to have this feature released?

Most likely immediately after I have this PR reviewed, though there are a couple of other things that I'd liked to be included.

if you can just add the table feature at the scenario level, it would be perfect

This feature already exists! Although it is labeled as an experimental feature (mostly because the reporting around this is to be implemented). See https://docs.gauge.org/writing-specifications.html#table-driven-scenario

Edit - I realize you've asked for <table:... > syntax for table driven scenario. This needs to be implemented, currently there is support for inline tables only.

@alpha1592
Copy link

@sriv Thanks for taking my feedback. I appreciate it. :)

Yes, the documentation currently states that CSV files or Table notation is not supported on the Scenario level.
The inline tables are of little use to us since our tables get really large (many rows and columns). We hold our test data and some of assertions in there as well. Plus inline tables are "hardcoded" which makes it difficult to run tests from env to env.

I would really like to be able to add different types of tests related to the same feature in one Test Specification.
For example, if testing login page of a web app, you can have the following scenarios..

  • Login Successful (table with many rows that represent various roles)
  • Invalid login attempts (table with few rows)
  • Test Forgot my password link (No table)
  • Test Register link (No table)

This is currently not possible if you have one CSV at the Specification level.
I know that feature is probably a different Feature request, but I wanted to bring attention to it.

Thanks

@sriv
Copy link
Member Author

sriv commented Jul 23, 2021

I know that feature is probably a different Feature request, but I wanted to bring attention to it.

Yes, that should be a separate feature request. TBH, I haven't assessed the magnitude of this change. Please do create an issue, so that we do not lose this discussion.

sriv added 3 commits July 23, 2021 17:10
…correct gauge_data_dir

Signed-off-by: sriv <srikanth.ddit@gmail.com>
Signed-off-by: sriv <srikanth.ddit@gmail.com>
fixed some tests/code lint

Signed-off-by: sriv <srikanth.ddit@gmail.com>
@sriv sriv marked this pull request as ready for review July 25, 2021 05:28
@sriv sriv requested a review from zabil July 25, 2021 05:28
util/fileUtils.go Outdated Show resolved Hide resolved
Co-authored-by: Zabil Cheriya Maliackal <zabil@users.noreply.github.com>
Signed-off-by: sriv <srikanth.ddit@gmail.com>
@sriv sriv force-pushed the 2046_env_specific_data branch from db03efd to 5662b07 Compare July 27, 2021 12:14
@sriv
Copy link
Member Author

sriv commented Jul 27, 2021

removed 'RC' label. This will be clubbed with #2057 in a single release.

@sriv sriv merged commit 3c341fb into master Jul 28, 2021
@sriv sriv deleted the 2046_env_specific_data branch July 28, 2021 00:29
sriv added a commit that referenced this pull request Jul 28, 2021
* introduce gauge_data_dir

allow csv/txt data files to be in subdirectory, configured by gauge_data_dir

fixes #2046

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* print debug when loading from gauge_data_dir, warning when setting incorrect gauge_data_dir

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* better error message when table is not resolved from gauge_data_dir

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* add tests for gauge_data_dir resolution

fixed some tests/code lint

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* Update util/fileUtils.go

Co-authored-by: Zabil Cheriya Maliackal <zabil@users.noreply.github.com>
Signed-off-by: sriv <srikanth.ddit@gmail.com>

Co-authored-by: Zabil Cheriya Maliackal <zabil@users.noreply.github.com>
sriv added a commit that referenced this pull request Jul 28, 2021
* allow custom environment directory, fixes #2025

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* bump version -> 1.4.0

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* introduce gauge_data_dir (#2047)

* introduce gauge_data_dir

allow csv/txt data files to be in subdirectory, configured by gauge_data_dir

fixes #2046

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* print debug when loading from gauge_data_dir, warning when setting incorrect gauge_data_dir

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* better error message when table is not resolved from gauge_data_dir

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* add tests for gauge_data_dir resolution

fixed some tests/code lint

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* Update util/fileUtils.go

Co-authored-by: Zabil Cheriya Maliackal <zabil@users.noreply.github.com>
Signed-off-by: sriv <srikanth.ddit@gmail.com>

Co-authored-by: Zabil Cheriya Maliackal <zabil@users.noreply.github.com>

* allow custom environment directory, fixes #2025

Signed-off-by: sriv <srikanth.ddit@gmail.com>

* fix merge

Signed-off-by: sriv <srikanth.ddit@gmail.com>

Co-authored-by: Zabil Cheriya Maliackal <zabil@users.noreply.github.com>
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.

Support for CSV file per environment.
3 participants