Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

#656 Support JavaScript setup/suite-setup/teardown/suite-teardown #657

Merged
merged 2 commits into from
Sep 14, 2016

Conversation

bradmann
Copy link
Contributor

@bradmann bradmann commented Sep 2, 2016

JavaScript is awesome.

@RobertSzkutak
Copy link
Contributor

RobertSzkutak commented Sep 6, 2016

Pinging @grtjn and @dmcassel for an opinion as I'm not very familiar with the testing framework.

I feel like the best way to handle this would be with a variable in src/test/test-config.xqy to pick whether you want XQY or SJS vs forcing XQY unless it fails and then forcing a try of SJS. Offhand, seems like that could throw a misleading error about SJS missing when really you're failing to invoke XQY? Could there ever be a scenario in which you would want to invoke both? Lastly, I think we would need sample SJS files in the repository to keep it newbie friendly.

On the surface, I think this absolutely benefits Roxy even if we would need to do some more work on it. I'll defer to someone else about whether our preferred process would be to first perfect the PR or merge and finish it later.

@dmcassel
Copy link
Collaborator

dmcassel commented Sep 6, 2016

I'll try to poke at it this week.

@dmcassel dmcassel self-assigned this Sep 6, 2016
@bradmann
Copy link
Contributor Author

bradmann commented Sep 6, 2016

I've already coded a solution and submitted a pull request against dev.

Sent from my Nexus 6P

On Sep 6, 2016 8:55 AM, "David Cassel" notifications@github.com wrote:

I'll try to poke at it this week.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#657 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAqxRWda_4NDNxr16Fj0MHfxXm0rVzbBks5qnWKzgaJpZM4JzUcJ
.

@dmcassel
Copy link
Collaborator

dmcassel commented Sep 6, 2016

Rephrasing -- I'll check out your PR this week

@dmcassel
Copy link
Collaborator

A couple things (nothing big):

  • on default.xqy line 456, there's a test to see whether a file should be considered a test. Since suite-setup.sjs and suite-teardown.sjs are not part of that test, they show up as unit tests and are executed as such (in addition to be run correctly as suite setup/teardown).
  • aren't filenames in JavaScript land more often camelCased? I'm thinking it would be more consistent to call these suiteSetup.sjs and suiteTeardown.sjs. (I know there isn't really a convention, I just think it makes sense to match the filename to what we do inside the file. Other opinions welcome, but absent consensus in the other direction, I'd like to see the filenames changed.)

Otherwise, this looks great.

change suite-teardown.sjs to suiteTeardown.sjs.
hide suiteSetup.sjs and suiteTeardown.sjs from display.
@bradmann
Copy link
Contributor Author

changes made.

@dmcassel dmcassel merged commit 24ec18c into marklogic-community:dev Sep 14, 2016
@dmcassel
Copy link
Collaborator

Thanks @bradmann

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants